Skip to content

Conversation

felixbarny
Copy link
Member

@felixbarny felixbarny commented Jul 23, 2019

  • Load the MDC with the application class loader, as opposed to the
    Thread's context classloader (fixes Stop using Thread context class loader for log correlation #720)
  • Remove span.id MDC entry
    This causes the most allocations as in contrast to transaction.id and
    trace.id, the String manifestation can't be reused.
    Other agents also don't add the span.id so it makes sense to align on that
  • Docs improvements
  • Mention slf4j in supported technologies page

closes #724

- Load the MDC with the application class loader, as opposed to the Thread's context classloader (fixes elastic#720) - Remove span.id MDC entry This causes the most allocations as in contrast to transaction.id and trace.id, the String manifestation can't be reused. Other agents also don't add the span.id so it makes sense to align on that - Docs improvements - Mention slf4j in supported technologies page
@felixbarny
Copy link
Member Author

run the tests

Copy link
Contributor

@eyalkoren eyalkoren left a comment

Choose a reason for hiding this comment

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

Comments and what we discussed regarding avoiding hard reference to the CL.
In addition, positive test for the fallback context CL.
And lastly- is there common agreement between agents whether the span ID is logged or not?

@codecov-io
Copy link

codecov-io commented Jul 25, 2019

Codecov Report

Merging #742 into master will increase coverage by 0.09%.
The diff coverage is 72.93%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #742 +/- ## ============================================ + Coverage 64.79% 64.89% +0.09%  + Complexity 169 68 -101  ============================================ Files 209 210 +1 Lines 8925 8955 +30 Branches 1147 1157 +10 ============================================ + Hits 5783 5811 +28  + Misses 2788 2784 -4  - Partials 354 360 +6
Impacted Files Coverage Δ Complexity Δ
...apm/agent/bci/bytebuddy/CustomElementMatchers.java 70.27% <ø> (-3.3%) 0 <0> (ø)
...lastic/apm/agent/report/ReporterConfiguration.java 100% <ø> (ø) 0 <0> (ø) ⬇️
...stic/apm/agent/jdbc/ConnectionInstrumentation.java 81.81% <0%> (-2.8%) 0 <0> (ø)
.../elastic/apm/agent/jdbc/helper/JdbcHelperImpl.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...tic/apm/agent/configuration/CoreConfiguration.java 97.72% <100%> (+0.77%) 0 <0> (ø) ⬇️
...astic/apm/agent/impl/transaction/AbstractSpan.java 83.92% <100%> (+0.29%) 0 <0> (ø) ⬇️
...a/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java 100% <100%> (ø) 0 <0> (?)
...a/co/elastic/apm/agent/report/ReporterFactory.java 100% <100%> (ø) 0 <0> (ø) ⬇️
...astic/apm/agent/report/ApmServerHealthChecker.java 96.55% <100%> (+31.33%) 0 <0> (ø) ⬇️
...lastic/apm/agent/impl/ElasticApmTracerBuilder.java 77.94% <100%> (+1.37%) 0 <0> (ø) ⬇️
... and 14 more

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 01400f2...5da8a67. Read the comment docs.

@felixbarny felixbarny merged commit a4f6c1c into elastic:master Jul 29, 2019
@felixbarny felixbarny deleted the log-correlation-ga branch July 29, 2019 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants