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

mess with tracing #9281

Merged
merged 1 commit into from
Apr 25, 2022
Merged

mess with tracing #9281

merged 1 commit into from
Apr 25, 2022

Conversation

AlexTugarev
Copy link
Member

@AlexTugarev AlexTugarev commented Apr 13, 2022

Trying to get rid of the millions of error logged in prod.

  • /werft with-observability=true
  • /werft with-helm=true
  • /werft with-clean-slate-deployment
NONE

@AlexTugarev
Copy link
Member Author

AlexTugarev commented Apr 13, 2022

/werft run

👍 started the job as gitpod-build-at-tracing.1

@AlexTugarev
Copy link
Member Author

AlexTugarev commented Apr 13, 2022

/werft run

👍 started the job as gitpod-build-at-tracing.2

})
: undefined;
let span;
if (!!spanCtx && !!spanCtx.toTraceId()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@geropl, I believe that's the root of the issue. traceId was missing.

}

if (msg) {
const spanCtx = globalTracer().extract(FORMAT_HTTP_HEADERS, message.properties.headers);
const span = !!spanCtx ? globalTracer().startSpan(`/messagebus/${this.exchangeName}`, { references: [childOf(spanCtx!)] }) : undefined;
let span;
if (!!spanCtx && !!spanCtx.toTraceId()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

traceId was missing here as well..

@AlexTugarev
Copy link
Member Author

AlexTugarev commented Apr 13, 2022

@geropl or @mads-hartmann, please help to get this verified.

Unfortunately, there are more changes included in this PR just to satisfy the checks, but I left a trace of comments to point to the intended ones.

Here is an example: https://ui.honeycomb.io/gitpod/datasets/preview-environments/result/4VSr1r5G3Rh/trace/koZ93EerQPm?span=995d31c40e8dd721

@geropl
Copy link
Member

geropl commented Apr 14, 2022

@AlexTugarev Thanks a ton for fixing this - the traces indeed look complete!

Please have a look at this comment. And then I'm happy to ✔️ 🏁 .

@geropl geropl self-assigned this Apr 14, 2022
@AlexTugarev AlexTugarev marked this pull request as ready for review April 14, 2022 09:53
@AlexTugarev AlexTugarev requested a review from a team April 14, 2022 09:53
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Apr 14, 2022
@AlexTugarev
Copy link
Member Author

@geropl, I added the spanId check.

Copy link
Member

@easyCZ easyCZ left a comment

Choose a reason for hiding this comment

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

The lint here obscures the nuance of the change. It'd make it easier for reviewers to first lint the file (can be done with pre-commit run --file <file>) and then base this on top, to highlight the changes made to tracing.

@AlexTugarev
Copy link
Member Author

AlexTugarev commented Apr 14, 2022

@easyCZ, you are absolutely right. 😿

@stale
Copy link

stale bot commented Apr 24, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Apr 24, 2022
@AlexTugarev AlexTugarev removed the meta: stale This issue/PR is stale and will be closed soon label Apr 25, 2022
@roboquat roboquat merged commit a57dee8 into main Apr 25, 2022
@roboquat roboquat deleted the at/tracing branch April 25, 2022 09:09
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Apr 26, 2022
@AlexTugarev AlexTugarev mentioned this pull request Apr 28, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/L team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants