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

fix: update opentelemetry.lua for envoy x-request-id format #10246

Closed

Conversation

artlin760402
Copy link

@artlin760402 artlin760402 commented Sep 22, 2023

Description

Remove all "-" to fulfill opentelemetry standard

Fixes #10239

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@Revolyssup Revolyssup changed the title Update opentelemetry.lua for envoy x-request-id format fix: update opentelemetry.lua for envoy x-request-id format Sep 22, 2023
@Revolyssup
Copy link
Contributor

Revolyssup commented Sep 22, 2023

@artlin760402 the change looks good but I have a question. The request will have - in the system where it was created like istio, but apisix would send it to collector without the -. Wouldn't it be tricky to then correlate the requests because collector receives the trace_id as non hyphenated but the actual trace_id is hyphenated?

@artlin760402
Copy link
Author

@artlin760402 the change looks good but I have a question. The request will have - in the system where it was created like istio, but apisix would send it to collector without the -. Wouldn't it be tricky to then correlate the requests because collector receives the trace_id as non hyphenated but the actual trace_id is hyphenated?

Yes it's a workaround solution for enterprise envoy environment. So it may needs extra serverless function plugin to overwrite header value, or the Devops might have to trace logs by manually remove hyphens...

@monkeyDluffy6017
Copy link
Contributor

Please make the ci pass

@monkeyDluffy6017 monkeyDluffy6017 added the wait for update wait for the author's response in this issue/PR label Sep 25, 2023
@Revolyssup
Copy link
Contributor

@artlin760402 Please fix the failing lint tests

@Sn0rt
Copy link
Contributor

Sn0rt commented Oct 27, 2023

image

it looks very different .
BTW: I think your should add a test case for your feature.

@shreemaan-abhishek
Copy link
Contributor

Directly removing hyphens from the trace-id isn't a correct way to fix the problem. If you want to fix the problem this way, you can use the serverless plugin to do it.

@artlin760402
Copy link
Author

Directly removing hyphens from the trace-id isn't a correct way to fix the problem. If you want to fix the problem this way, you can use the serverless plugin to do it.

Yes, agreed

@monkeyDluffy6017
Copy link
Contributor

@artlin760402 I will close this pr, looking forward to your next fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user responded wait for update wait for the author's response in this issue/PR
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

help request: How to use x-request-id as trace_id with apisix opentelemetry over istio service mesh
5 participants