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

feat: prefer 'traceparent' header over 'elastic-apm-traceparent' header #2079

Merged
merged 3 commits into from
May 27, 2021

Conversation

trentm
Copy link
Member

@trentm trentm commented May 19, 2021

The w3c traceparent header is standardized and widely used enough now in Elastic tools that we should prefer it over the proprietary 'elastic-apm-traceparent' header.

See similar for Java, .NET.

Checklist

  • Implement code
  • Add CHANGELOG.asciidoc entry
  • Commit message follows commit guidelines

@trentm trentm requested a review from astorm May 19, 2021 17:53
@trentm trentm self-assigned this May 19, 2021
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label May 19, 2021
@apmmachine
Copy link
Contributor

apmmachine commented May 19, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #2079 updated

  • Start Time: 2021-05-26T22:17:47.193+0000

  • Duration: 19 min 7 sec

  • Commit: 95580cd

Test stats 🧪

Test Results
Failed 0
Passed 21428
Skipped 0
Total 21428

Trends 🧪

Image of Build Times

Image of Tests

astorm
astorm previously approved these changes May 19, 2021
Copy link
Contributor

@astorm astorm left a comment

Choose a reason for hiding this comment

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

Approving pending CI passing.

Just talking through the consequences of this out loud, there's a theoretical change in behavior if a user is setting both of these headers and we start preferring the W3C trace header over the old proprietary header.

However, this doesn't seem like anything that should cause a breaking change -- the behavior change will be better interoperability with the standards based header.

The only users who will see a degradation in their experience are folks who might have some services that add both headers, and other services that only add the old proprietary header. That seems like an edge case on an edge case (and they can always remain on an older agent), so I don't think we'll need a major version change for this change.

@astorm
Copy link
Contributor

astorm commented May 24, 2021

jenkins run the tests

@astorm
Copy link
Contributor

astorm commented May 24, 2021

jenkins run the tests please

@trentm trentm requested a review from astorm May 27, 2021 15:41
@trentm trentm merged commit f0a42c3 into master May 27, 2021
@trentm trentm deleted the trentm/deprecate-elastic-apm-traceparent-header branch May 27, 2021 15:49
dgieselaar pushed a commit to dgieselaar/apm-agent-nodejs that referenced this pull request Sep 10, 2021
…er (elastic#2079)

When using traceparent info from headers of an incoming request, prefer the 
W3C "traceparent" header to Elastic's pre-W3C Trace Context spec header.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants