Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for tracing via OpenTelemetry #254

Merged
merged 13 commits into from Nov 28, 2022

Conversation

josephwoodward
Copy link
Contributor

@josephwoodward josephwoodward commented Oct 17, 2022

As OpenTracing (the predecessor to OpenTelemetry) has been deprecated, this change adds support for OpenTelemetry via the WithOpenTelemetry functional option.

A lot of the API and options align with the otelhttp transport that can be used on the net/http client

Addresses issue #249

@josephwoodward josephwoodward changed the title Add support for Open Telemetry Add support for OpenTelemetry Oct 17, 2022
@josephwoodward josephwoodward changed the title Add support for OpenTelemetry Add support for tracing via OpenTelemetry Oct 17, 2022
Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
attribute.String(string(semconv.HTTPMethodKey), op.Method),
attribute.String("span.kind", trace.SpanKindClient.String()),
//TODO: Figure out the best way to get the scheme?
attribute.String("http.scheme", op.Schemes[0]),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @casualjim, can you offer any pointers on the best way to get the scheme? I don't appear to have access to the underlying http.Request object to check the existence of the TLS field (which is one common way to work out the scheme)

Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
@josephwoodward josephwoodward marked this pull request as ready for review November 4, 2022 06:55
@casualjim
Copy link
Member

Thanks for all your work on this.

Copy link
Member

@casualjim casualjim left a comment

Choose a reason for hiding this comment

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

This looks good.

I'll see if I can figure out how to get the actual scheme that's being used for the request into here too.

@casualjim
Copy link
Member

can you bump the go version in the .github/ folder to be at least 1.18 or higher? Otel seems to require it

Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
@josephwoodward
Copy link
Contributor Author

@casualjim That's been bumped, do you want me to bump the version in go.mod too?

@casualjim
Copy link
Member

yeah that would be good

Signed-off-by: Joseph Woodward <joseph.woodward@xeuse.com>
@yvvq
Copy link

yvvq commented Nov 28, 2022

@casualjim Could you please merge this pr if It’s ok with you.

@codecov
Copy link

codecov bot commented Nov 28, 2022

Codecov Report

Merging #254 (f454d66) into master (53ffffd) will increase coverage by 0.07%.
The diff coverage is 89.91%.

@@            Coverage Diff             @@
##           master     #254      +/-   ##
==========================================
+ Coverage   80.13%   80.20%   +0.07%     
==========================================
  Files          42       44       +2     
  Lines        3191     3350     +159     
==========================================
+ Hits         2557     2687     +130     
- Misses        521      548      +27     
- Partials      113      115       +2     
Impacted Files Coverage Δ
client/runtime.go 62.16% <0.00%> (-1.38%) ⬇️
client/opentelemetry.go 91.45% <91.45%> (ø)
middleware/context.go 72.55% <0.00%> (-1.00%) ⬇️
client/response.go 100.00% <0.00%> (ø)
middleware/swaggerui_oauth2.go 72.00% <0.00%> (ø)
middleware/swaggerui.go 87.50% <0.00%> (+0.70%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@casualjim casualjim merged commit 93d335a into go-openapi:master Nov 28, 2022
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.

None yet

3 participants