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

fix(otel): Better handling for HTTP span attributes #610

Merged
merged 3 commits into from Mar 30, 2023

Conversation

@codecov
Copy link

codecov bot commented Mar 29, 2023

Codecov Report

Patch coverage: 75.00% and project coverage change: +1.01 🎉

Comparison is base (8fdc323) 78.91% compared to head (b4d468a) 79.92%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #610      +/-   ##
==========================================
+ Coverage   78.91%   79.92%   +1.01%     
==========================================
  Files          38       38              
  Lines        3851     3861      +10     
==========================================
+ Hits         3039     3086      +47     
+ Misses        710      669      -41     
- Partials      102      106       +4     
Impacted Files Coverage Δ
otel/internal/utils/spanattributes.go 47.66% <75.00%> (+40.44%) ⬆️

... and 1 file with indirect coverage changes

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

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tonyo tonyo self-assigned this Mar 29, 2023
@tonyo tonyo added Status: In Progress Topic: OpenTelemetry Issue/PR related to OpenTelemetry integration labels Mar 29, 2023
@tonyo tonyo marked this pull request as ready for review March 29, 2023 14:44
// This is normally the HTTP-client case
if parsedUrl, err := url.Parse(httpUrl); err == nil {
// Do not include the query and fragment parts
httpPath = fmt.Sprintf("%s://%s%s", parsedUrl.Scheme, parsedUrl.Host, parsedUrl.Path)
Copy link
Member Author

Choose a reason for hiding this comment

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

After chatting with @antonpirker I decided to drop the query/fragment parts, just to be on a bit safer side in terms of both cardinality and PII.

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

Nice, thanks Anton!

@tonyo tonyo merged commit a97bb8b into master Mar 30, 2023
13 of 15 checks passed
@tonyo tonyo deleted the tonyo/http-otel-attrs-2 branch March 30, 2023 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: In Progress Topic: OpenTelemetry Issue/PR related to OpenTelemetry integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OTel] Improve HTTP Span attributes in SpanProcessor
3 participants