Skip to content

Conversation

@mmoustai
Copy link

No description provided.

@mmoustai mmoustai changed the title feat(opentracing): disable tracing error on middleware disable tracing error on middleware May 13, 2019
@mmoustai mmoustai changed the title disable tracing error on middleware Disable tracing error on middleware May 13, 2019
@codecov-io
Copy link

codecov-io commented May 13, 2019

Codecov Report

Merging #209 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #209   +/-   ##
=======================================
  Coverage   73.09%   73.09%           
=======================================
  Files          36       36           
  Lines        1349     1349           
=======================================
  Hits          986      986           
  Misses        314      314           
  Partials       49       49
Impacted Files Coverage Δ
tracing/opentracing/server_interceptors.go 79.06% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfaf568...f55fe64. Read the comment docs.

Copy link
Contributor

@domgreen domgreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @mmoustai if we are looking to disable the tracing of errors can we do so via opts into the interceptor and keep the default to true so that it does not unexpectedly change the default behavior for others.

Could you also look to add a test and example for the new opt. Thanks :)

newCtx, serverSpan := newServerSpanFromInbound(ctx, o.tracer, info.FullMethod)
resp, err := handler(newCtx, req)
finishServerSpan(ctx, serverSpan, err)
finishServerSpan(ctx, serverSpan, err, false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets pass this via opts ... maybe something like DisableErrorTracing default to true so not to change behaviour

}

func finishServerSpan(ctx context.Context, serverSpan opentracing.Span, err error) {
func finishServerSpan(ctx context.Context, serverSpan opentracing.Span, err error, active bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

active > o.DisableErrorTracing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants