Skip to content

Remap http.response.status_code to http.status_code for otel spans#4384

Merged
zarirhamza merged 3 commits intomasterfrom
zarir/otel-http-remap-tag
Jun 7, 2024
Merged

Remap http.response.status_code to http.status_code for otel spans#4384
zarirhamza merged 3 commits intomasterfrom
zarir/otel-http-remap-tag

Conversation

@zarirhamza
Copy link
Copy Markdown
Contributor

@zarirhamza zarirhamza commented Jun 6, 2024

What does this PR do?

Remap http.response.status_code to http.status_code

Motivation

To make otel spans compliant with trace metrics

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 6, 2024 •
edited
Loading

Overall package size

Self size: 6.62 MB
Deduped: 61.88 MB
No deduping: 62.16 MB

Dependency sizes

name version self size total size
@datadog/native-appsec 8.0.1 15.59 MB 15.6 MB
@datadog/native-iast-taint-tracking 2.1.0 14.91 MB 14.92 MB
@datadog/pprof 5.3.0 9.85 MB 10.22 MB
protobufjs 7.2.5 2.77 MB 6.56 MB
@datadog/native-iast-rewriter 2.3.1 2.15 MB 2.24 MB
@opentelemetry/core 1.14.0 872.87 kB 1.47 MB
@datadog/native-metrics 2.0.0 898.77 kB 1.3 MB
@opentelemetry/api 1.8.0 1.21 MB 1.21 MB
import-in-the-middle 1.7.4 70.19 kB 739.86 kB
msgpack-lite 0.1.26 201.16 kB 281.59 kB
opentracing 0.14.7 194.81 kB 194.81 kB
semver 7.5.4 93.4 kB 123.8 kB
pprof-format 2.1.0 111.69 kB 111.69 kB
@datadog/sketches-js 2.1.0 109.9 kB 109.9 kB
lodash.sortby 4.7.0 75.76 kB 75.76 kB
lru-cache 7.14.0 74.95 kB 74.95 kB
ignore 5.2.4 51.22 kB 51.22 kB
int64-buffer 0.1.10 49.18 kB 49.18 kB
shell-quote 1.8.1 44.96 kB 44.96 kB
istanbul-lib-coverage 3.2.0 29.34 kB 29.34 kB
tlhunter-sorted-set 0.1.0 24.94 kB 24.94 kB
limiter 1.1.5 23.17 kB 23.17 kB
dc-polyfill 0.1.4 23.1 kB 23.1 kB
retry 0.13.1 18.85 kB 18.85 kB
jest-docblock 29.7.0 8.99 kB 12.76 kB
crypto-randomuuid 1.0.0 11.18 kB 11.18 kB
path-to-regexp 0.1.7 6.78 kB 6.78 kB
koalas 1.0.2 6.47 kB 6.47 kB
module-details-from-path 1.0.3 4.47 kB 4.47 kB

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Jun 6, 2024 •
edited
Loading

Benchmarks

Benchmark execution time: 2024-06-07 15:25:27

Comparing candidate commit 5586d6f in PR branch zarir/otel-http-remap-tag with baseline commit 929cc16 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 258 metrics, 8 unstable metrics.

tlhunter
tlhunter requested changes Jun 6, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@tlhunter tlhunter left a comment

Choose a reason for hiding this comment

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

This change modifies a function that's a bit more "global" in usage. Specifically setAttribute is used not just by HTTP but also by databases, queues, gRPC, etc.

Maintaining this list would be a little painful, too. Imagine if we had a dozen of them. Plus there's a small performance penalty for adding an if statement in a code hot path.

Instead I'd suggest looking directly at the source for writing HTTP related tags. Check out packages/datadog-plugin-http2/src/client.js and grep for HTTP_STATUS_CODE for a more "local" place to change the code.

@zarirhamza zarirhamza changed the title Remap http.response.status_code to http.status_code Remap http.response.status_code to http.status_code for otel spans Jun 6, 2024
@zarirhamza zarirhamza marked this pull request as ready for review June 6, 2024 17:03
@zarirhamza zarirhamza requested a review from a team as a code owner June 6, 2024 17:03
@tlhunter
Copy link
Copy Markdown
Member

tlhunter commented Jun 6, 2024

This change modifies a function that's a bit more "global" in usage. Specifically setAttribute is used not just by HTTP but also by databases, queues, gRPC, etc.

Maintaining this list would be a little painful, too. Imagine if we had a dozen of them. Plus there's a small performance penalty for adding an if statement in a code hot path.

Instead I'd suggest looking directly at the source for writing HTTP related tags. Check out packages/datadog-plugin-http2/src/client.js and grep for HTTP_STATUS_CODE for a more "local" place to change the code.

I didn't realize this was for setting otel attributes, not our regular tags.

@zarirhamza zarirhamza enabled auto-merge (squash) June 6, 2024 17:06
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 7, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.86%. Comparing base (02dc5ce) to head (5586d6f).
Report is 2 commits behind head on master.

❗ Current head 5586d6f differs from pull request most recent head 02dfa8e

Please upload reports for the commit 02dfa8e to get more accurate results.

Additional details and impacted files 
@@             Coverage Diff             @@
##           master    #4384       +/-   ##
===========================================
+ Coverage   69.19%   81.86%   +12.67%     
===========================================
  Files           1      120      +119     
  Lines         198     4621     +4423     
  Branches       33       33               
===========================================
+ Hits          137     3783     +3646     
- Misses         61      838      +777

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zarirhamza zarirhamza merged commit 410078e into master Jun 7, 2024
@zarirhamza zarirhamza deleted the zarir/otel-http-remap-tag branch June 7, 2024 15:25
ida613 pushed a commit that referenced this pull request Jun 7, 2024
@zarirhamza @ida613
remaps http status tag to meet ddog convention for otel(#4384)
f46f75f
@ida613 ida613 mentioned this pull request Jun 7, 2024
Merged
ida613 pushed a commit that referenced this pull request Jun 7, 2024
@zarirhamza @ida613
remaps http status tag to meet ddog convention for otel(#4384)
a34ff06
@ida613 ida613 mentioned this pull request Jun 7, 2024
Merged
ida613 pushed a commit that referenced this pull request Jun 10, 2024
@zarirhamza @ida613
remaps http status tag to meet ddog convention for otel(#4384)
321451c
ida613 added a commit that referenced this pull request Jun 10, 2024
@ida613
Revert "remaps http status tag to meet ddog convention for otel(#4384)"
4f0d455 
This reverts commit 321451c.
ida613 pushed a commit that referenced this pull request Jun 10, 2024
@zarirhamza @ida613
remaps http status tag to meet ddog convention for otel(#4384)
4fa2520
@erikvanbrakel erikvanbrakel mentioned this pull request Jul 5, 2024
Open
@bburns bburns mentioned this pull request Jul 12, 2024
Open
@Kenno28 Kenno28 mentioned this pull request Jul 16, 2024
Open
@Pyloris Pyloris mentioned this pull request Jul 31, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tlhunter tlhunter tlhunter approved these changes

Assignees

No one assigned

Labels

open-telemetry semver-minor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zarirhamza @tlhunter