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.

Sign up for GitHub

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

Jump to bottom

Prefer w3c traceparent header over legacy elastic-apm-traceparent #442

Merged
merged 2 commits into from
Jun 7, 2021
Merged

Prefer w3c traceparent header over legacy elastic-apm-traceparent #442

merged 2 commits into from
Jun 7, 2021

Conversation

felixbarny
Copy link
Member

@felixbarny felixbarny commented May 26, 2021 •
edited by AlexanderWert
Loading

@AlexanderWert:

Whenever there is something in the middle (let's say a reverse proxy) of two services being monitored by our agent. And the proxy uses the W3C headers to create own spans (--> thus sends modified traceparent header to the downstream service), but passes our proprietary header as is. In this case the downstream service has a conflict, especially if it prefers the proprietary header over the W3C header, resulting in wrong parent child relationship.

Status

Agent Status - Issue / PR
Java Custom badge
.NET Custom badge
Node.js Custom badge
Python Custom badge
Go ✅ (already in correct order)
Ruby ✅ (already in correct order)
PHP ✅ (supports only W3C header)

@felixbarny felixbarny requested review from a team as code owners May 26, 2021 07:11
mikker
mikker approved these changes May 26, 2021
View reviewed changes
Copy link
Contributor

@mikker mikker left a comment

Choose a reason for hiding this comment

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

Ruby Agent already does this 👍

@apmmachine
Copy link

apmmachine commented May 26, 2021 •
edited
Loading

💚 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 #442 updated

  • Start Time: 2021-06-07T13:13:41.814+0000

  • Duration: 2 min 58 sec

  • Commit: f35d46f

Trends 🧪

Image of Build Times

This was referenced May 26, 2021
Merged
Merged
trentm
trentm approved these changes May 27, 2021
View reviewed changes
Copy link
Member

@trentm trentm left a comment

Choose a reason for hiding this comment

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

Just merged a change for this to the Node.js APM agent: elastic/apm-agent-nodejs#2079

@axw axw mentioned this pull request May 28, 2021
Merged
@felixbarny felixbarny enabled auto-merge (squash) June 7, 2021 13:15
@felixbarny felixbarny merged commit 8cf5710 into master Jun 7, 2021
@felixbarny felixbarny deleted the prefer-w3c-header branch June 7, 2021 13:16
@AlexanderWert AlexanderWert mentioned this pull request Jun 7, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikker mikker mikker approved these changes

@trentm trentm trentm approved these changes

@beniwohli beniwohli beniwohli approved these changes

@axw axw axw approved these changes

@gregkalapos gregkalapos gregkalapos approved these changes

@hmdhk hmdhk hmdhk approved these changes

@eyalkoren eyalkoren eyalkoren approved these changes

@alex-fedotyev alex-fedotyev alex-fedotyev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@felixbarny @apmmachine @mikker @trentm @beniwohli @axw @gregkalapos @hmdhk @eyalkoren @alex-fedotyev