WIP: #260: add open-tracing support #278
Conversation
maihacke
left a comment
maihacke
left a comment
There was a problem hiding this comment.
I made a few comments to the code. On top of that I see the following points:
- There is obviously some clean up needed for the commented out code
- I'm missing the part for http/rest which parses the http-headers and initializes the tracing with the values (e.g. for logging).
| <dependency> | ||
| <groupId>io.opentracing</groupId> | ||
| <artifactId>opentracing-api</artifactId> | ||
| <version>0.33.0</version> |
There was a problem hiding this comment.
This version should go to the bom
| <dependency> | ||
| <groupId>io.jaegertracing</groupId> | ||
| <artifactId>jaeger-client</artifactId> | ||
| <version>0.32.0</version> |
There was a problem hiding this comment.
This version should go to the bom
| Tags.ERROR.set(span, true); | ||
| span.log(Map.of(Fields.EVENT, "error", Fields.ERROR_OBJECT, ex, Fields.MESSAGE, ex.getMessage())); | ||
| } finally { | ||
| span.finish(); |
There was a problem hiding this comment.
I do not get the point here? why is the span immediatly closed after setting the headers? Should the span be closed after the http request is made?
There was a problem hiding this comment.
@maihacke i thought a for each rest call, the request comes to this ServletHeaderContext class. so the setting the span to the headers and closing it. for the next call , new span is created , logged, set to the headers and closed.
|
Ravichandran M seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
3 participants
PR for issue #260 (open tracing)