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

New Feature - Add the traceid in the response headers #280

Closed
rider11-dev opened this issue Mar 17, 2018 · 3 comments · Fixed by #286
Closed

New Feature - Add the traceid in the response headers #280

rider11-dev opened this issue Mar 17, 2018 · 3 comments · Fixed by #286
Labels
feature A new feature good first issue Should be pretty easy to do help wanted Not actively being worked on. If you plan to contribute, please drop a note. small effort Likely less than a day of development effort.

Comments

@rider11-dev
Copy link

Hi,we are using Butterfly in Ocelot for tracing.
Generally,when client request fails,we want to find the exact trace quickly.
I think it is fastest to search a trace by traceId ,because it matches only one trace.
So could you add the traceid in the response headers when using tracing before return?
Just like this below:
responseMessage.Headers.TryAddWithoutValidation("TraceId", span.SpanContext.TraceId);

Ocelot/src/Ocelot/Requester/OcelotHttpTracingHandler.cs

Line 58 in c1b3151


Thanks!
@TomPallister TomPallister added feature A new feature help wanted Not actively being worked on. If you plan to contribute, please drop a note. good first issue Should be pretty easy to do small effort Likely less than a day of development effort. labels Mar 17, 2018
@TomPallister
Copy link
Member

TomPallister commented Mar 17, 2018 •
edited
Loading

@zpf1989 That sounds fine to me but I would probably make it configurable. Some users might not want to expose the trace ID. Maybe extend this feature
http://ocelot.readthedocs.io/en/latest/features/headerstransformation.html#placeholders.

e.g..

"DownstreamHeaderTransform": {
    "Trace-Id": "Trace-Id, {TraceId}"
},

This would then add the TraceId as a header called Trace-Id but Trace-Id could be whatever you want. I will look into it asap!

@rider11-dev
Copy link
Author

Yeah,that's considerate!
Thanks!

@TomPallister TomPallister changed the title Advice-Add the traceid in the response headers Few Feature - Add the traceid in the response headers Mar 17, 2018
@TomPallister TomPallister changed the title Few Feature - Add the traceid in the response headers New Feature - Add the traceid in the response headers Mar 17, 2018
TomPallister pushed a commit that referenced this issue Mar 17, 2018
#280 can now add response headers inc trace id, now need to consolida…
06f5849 
…te the header place holder stuff
@TomPallister TomPallister mentioned this issue Mar 17, 2018
Merged
TomPallister pushed a commit that referenced this issue Mar 18, 2018
#280 changed port for linux tests
787a85a
TomPallister pushed a commit that referenced this issue Mar 18, 2018
#280 lots of hacking around to handle errors and consolidate placehol…
8f88c08 
…ders into one class
TomPallister added a commit that referenced this issue Mar 18, 2018
@TomPallister
#280 Add headers to response (#286)
b51df71 
* #280 can now add response headers inc trace id, now need to consolidate the header place holder stuff

* #280 changed port for linux tests

* #280 lots of hacking around to handle errors and consolidate placeholders into one class
@TomPallister TomPallister reopened this Mar 18, 2018
@TomPallister
Copy link
Member

released in version 5.1.0

@FelixBoers FelixBoers mentioned this issue Apr 13, 2018
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature A new feature good first issue Should be pretty easy to do help wanted Not actively being worked on. If you plan to contribute, please drop a note. small effort Likely less than a day of development effort.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

#280 Add headers to response ThreeMammals/Ocelot
2 participants
@TomPallister @rider11-dev