-
Notifications
You must be signed in to change notification settings - Fork 43
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
Manually add OTEL context to both root spans #807
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: saschagrunert The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
63c0164
to
bb2a9b5
Compare
According to open-telemetry/opentelemetry-rust#888 (comment), this works around the issue where the root span is missing from the traces. Signed-off-by: Sascha Grunert <[email protected]>
bb2a9b5
to
9240f2f
Compare
@@ -312,7 +312,7 @@ var _ = Describe("ConmonClient", func() { | |||
rssAfter := vmRSSGivenPID(pid) | |||
GinkgoWriter.Printf("VmRSS after: %d\n", rssAfter) | |||
GinkgoWriter.Printf("VmRSS diff: %d\n", rssAfter-rssBefore) | |||
Expect(rssAfter - rssBefore).To(BeNumerically("<", 1200)) | |||
Expect(rssAfter - rssBefore).To(BeNumerically("<", 1500)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The integration tests seemed to flake more lately. Not really related to this PR, but should make them more resilient.
/lgtm |
Tide seems to not react on the state of this PR 🤔 |
What type of PR is this?
/kind bug
What this PR does / why we need it:
According to open-telemetry/opentelemetry-rust#888 (comment), this works around the issue where the root span is missing from the traces.
Which issue(s) this PR fixes:
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?