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

Add option to return the traceId to the API user #8124

Closed
r1ckr opened this issue Aug 24, 2020 · 10 comments
Closed

Add option to return the traceId to the API user #8124

r1ckr opened this issue Aug 24, 2020 · 10 comments
Labels
plugins/zipkin stale/pending revisit Too old ticket. Closed, but we may revisit later. task/feature Requests for new features in Kong

Comments

@r1ckr
Copy link

r1ckr commented Aug 24, 2020

Would be nice to be able to return the trace id back to the API user as an X-Request-Id or X-Correlation-Id header. The same way that the correlation id plugin does.

Something like:
config.external_http_header_name=X-Request-Id

If set, it would send back to the users the trace id in it.

@jcchavezs
Copy link
Contributor

jcchavezs commented Aug 24, 2020

I think this is in the scope of another plugin as it has nothing to do with either distributed tracing or zipkin. That said, this is a good feature request for kong.

@r1ckr
Copy link
Author

r1ckr commented Aug 24, 2020

If the zipkin plugin had the chance to echo the header back to the API user, we could use the response transformer plugin to rename that header to whatever we want and no new plugin is needed. That could look exactly like the correlation id plugin:

config.echo_downstream=true|false

If true Kong returns the x-b3-traceid header and then we can use the response transformer to rename it to X-Request-Id or whatever other name we would like to have.

@C13490888
Copy link

This is a useful feature request, would like to see something like this added!

@jcchavezs
Copy link
Contributor

I don't doubt it is useful but I am not sure it belongs here. Returning x-b3-traceid sounds reasonable but returning other headers does not and I believe adding that feature here would just add technical debt and an unrelated feature to maintain.

@ecourreges-orange
Copy link

It means here that the client code is "evolved enough" to read the returned x-b3-traceid and print it in its logs without making the extra effort to generate a traceid himself.
In case of tracing, you also have the sampling problem where you will be able to correlate logs of all apps, but you have a lower chance of finding that traceid in your tracing GUI.

Maybe your requirement @r1ckr could be an addendum to the correlation id plugin which could use the trace-id as correlation-id from the zipkin-plugin if present, and generate it if not?

@jcchavezs
Copy link
Contributor

jcchavezs commented Aug 31, 2020 via email

@r1ckr
Copy link
Author

r1ckr commented Aug 31, 2020

Folks, what we are trying to do here is to return the x-b3-traceid to the API consumer as the Request-Id header that we can see in many APIs. This Request-Id header is indeed used for troubleshooting API calls that went wrong.

If we use the correlation-id plugin, it will indeed return the x-b3-traceid header back to the API consumer, but no one would like to return a header like that back to the public for obvious reasons.

So, because of that we can go ahead and use the response-transformer plugin to rename that header, but it doesn't work, because the execution order of the correlation-id plugin is 1. So response-transformer happens before.

If the zipkin plugin was able to echo this header back to the public, then we would be able to use response-transformer to rename that header to whatever we want.

What you guys are proposing is to create a dependency in the correlation-id plugin, to configure it to read the trace-id as the correlation-id from the zipkin plugin if present?

Do you thing that's better than having a config here to echo out the x-b3-traceid header? Which has no dependencies and therefore easier to test

@jcchavezs
Copy link
Contributor

If we use the correlation-id plugin, it will indeed return the x-b3-traceid header back to the API consumer, but no one would like to return a header like that back to the public for obvious reasons.

Just out of curiosity, what are the reasons?

Do you thing that's better than having a config here to echo out the x-b3-traceid header? Which has no dependencies and therefore easier to test

I still think this could perfectly happen in this plugin.

@r1ckr
Copy link
Author

r1ckr commented Sep 1, 2020

Just out of curiosity, what are the reasons?

Well, convention headers for this type of information are Correlation-Id and Request-id and you also don't want to expose to the public your implementation on what are you using to share tracing context.

I still think this could perfectly happen in this plugin.

By this plugin you mean, zipkin or correlation-id?

@gszr gszr transferred this issue from Kong/kong-plugin-zipkin Nov 30, 2021
@gszr gszr added plugins/zipkin task/feature Requests for new features in Kong labels Nov 30, 2021
fffonion pushed a commit that referenced this issue Oct 18, 2022
This PR adds a new field in Zipkin plugin schema to support adding trace id in the response header.

Fix FTI-4198, #8124
@StarlightIbuki StarlightIbuki added the stale/pending revisit Too old ticket. Closed, but we may revisit later. label Oct 11, 2023
@StarlightIbuki
Copy link
Contributor

Dear contributor,
We're closing this issue as there hasn't been any update to it for a long time. If the issue is still relevant in the latest version, please feel free to reopen it. We're more than happy to revisit it again. Your contribution is greatly appreciated!
Please have a look at our pledge to the community for more information.
Sincerely,
Kong Gateway Team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugins/zipkin stale/pending revisit Too old ticket. Closed, but we may revisit later. task/feature Requests for new features in Kong
Projects
None yet
Development

No branches or pull requests

6 participants