Skip to content
This repository was archived by the owner on Jul 1, 2022. It is now read-only.

Conversation

@avimas
Copy link
Contributor

@avimas avimas commented Jun 28, 2020

Which problem is this PR solving?

documentation for #718

Short description of the changes

add documentation for log correlation

Signed-off-by: avim <avi.maslati@forescout.com>
@avimas avimas force-pushed the mdc-support-documentation branch from 4d594f9 to 4971e72 Compare June 28, 2020 12:01
@codecov
Copy link

codecov bot commented Jun 28, 2020

Codecov Report

Merging #724 into master will increase coverage by 0.22%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #724      +/-   ##
============================================
+ Coverage     88.76%   88.98%   +0.22%     
- Complexity      596      598       +2     
============================================
  Files            73       73              
  Lines          2242     2242              
  Branches        289      289              
============================================
+ Hits           1990     1995       +5     
+ Misses          159      155       -4     
+ Partials         93       92       -1     
Impacted Files Coverage Δ Complexity Δ
...gertracing/internal/reporters/LoggingReporter.java 90.90% <0.00%> (+9.09%) 5.00% <0.00%> (+1.00%)
...rtracing/internal/reporters/CompositeReporter.java 100.00% <0.00%> (+28.57%) 7.00% <0.00%> (+1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 936b39b...e7fb8d7. Read the comment docs.

@avimas avimas changed the title document log correlation Document MDC log correlation support Jun 28, 2020
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

Thanks! A few minor suggestions.

Comment on lines 145 to 150
The Jaeger Java Client also provides log correlation support by configuring the logging context (MDC)
with the following variables:

trace id - %{traceId}
span id - %{spanId}
sampled - %{sampled}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The Jaeger Java Client also provides log correlation support by configuring the logging context (MDC)
with the following variables:
trace id - %{traceId}
span id - %{spanId}
sampled - %{sampled}
The Jaeger Java Client supports log correlation by updating the logging context (MDC) with three variables: `traceId`, `spanId`, `sampled`.


To accomplish that Jaegar Tracer should be created with the following two steps:

1. Create the MDCScopeManager using either default names:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. Create the MDCScopeManager using either default names:
1. Create the MDCScopeManager using either default MDC field names:

2. Create the Jaegar Tracer by supplying the MDCScopeManager created step 1:
```java
JaegerTracer.Builder("serviceName").withScopeManager(scopeManager).build();
```
Copy link
Member

Choose a reason for hiding this comment

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

I would also add that the log formatter must be configured to include MDC fields in the output. Show example of the formatter for, say, log4j, and include a sample output log line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback!

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

thanks!

@yurishkuro yurishkuro merged commit 594b23c into jaegertracing:master Jun 28, 2020
@avimas avimas deleted the mdc-support-documentation branch June 29, 2020 08:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants