Skip to content

Conversation

@tobiasstadler
Copy link
Contributor

@tobiasstadler tobiasstadler commented Apr 1, 2021

What does this PR do?

Fixes #1735

Checklist

  • This is an enhancement of existing features, or a new feature in existing plugins
    • I have updated CHANGELOG.asciidoc
    • I have added tests that prove my fix is effective or that my feature works
    • Added an API method or config option? Document in which version this will be introduced
    • I have made corresponding changes to the documentation
  • This is a bugfix
  • This is a new plugin
    • I have updated CHANGELOG.asciidoc
    • My code follows the style guidelines of this project
    • I have made corresponding changes to the documentation
    • I have added tests that prove my fix is effective or that my feature works
    • New and existing unit tests pass locally with my changes
    • I have updated supported-technologies.asciidoc
    • Added an API method or config option? Document in which version this will be introduced
    • Added an instrumentation plugin? Describe how you made sure that old, non-supported versions are not instrumented by accident.
  • This is something else
@tobiasstadler
Copy link
Contributor Author

I would consider this a breaking change. If you agree/disagree I will update the changeling accordingly.

@ghost
Copy link

ghost commented Apr 1, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: SylvainJuge commented: jenkins run the tests please

  • Start Time: 2021-04-12T14:49:03.421+0000

  • Duration: 52 min 6 sec

  • Commit: 5cc7723

Test stats 🧪

Test Results
Failed 0
Passed 1982
Skipped 19
Total 2001

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 1982
Skipped 19
Total 2001

@SylvainJuge SylvainJuge self-assigned this Apr 6, 2021
@SylvainJuge
Copy link
Member

jenkins run the tests please

@codecov-io
Copy link

Codecov Report

Merging #1738 (5cc7723) into master (d7de9d2) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #1738 +/- ## ============================================ + Coverage 61.13% 61.17% +0.03%  - Complexity 3607 3610 +3  ============================================ Files 402 402 Lines 18294 18297 +3 Branches 2509 2509 ============================================ + Hits 11184 11193 +9  + Misses 6323 6318 -5  + Partials 787 786 -1 
Impacted Files Coverage Δ Complexity Δ
...apm/agent/bci/bytebuddy/CustomElementMatchers.java 70.27% <100.00%> (+3.12%) 15.00 <1.00> (+1.00)
...bci/methodmatching/TraceMethodInstrumentation.java 59.61% <100.00%> (-2.89%) 10.00 <0.00> (ø)
...pm/agent/pluginapi/CaptureSpanInstrumentation.java 40.74% <100.00%> (+2.27%) 7.00 <0.00> (ø)
...t/pluginapi/CaptureTransactionInstrumentation.java 32.25% <100.00%> (+2.25%) 6.00 <0.00> (ø)
...tic/apm/agent/pluginapi/TracedInstrumentation.java 27.02% <100.00%> (+2.02%) 6.00 <0.00> (ø)
...ic/apm/agent/profiler/collections/LongHashSet.java 17.85% <0.00%> (+1.98%) 13.00% <0.00%> (+2.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 d7de9d2...5cc7723. Read the comment docs.

@SylvainJuge
Copy link
Member

After discussing this internally we agreed that it should not really be a breaking change as it mostly impacts transaction/span naming. Also, it's very unlikely that there would be any strong dependency on current names as those are mostly generated at runtime, and thus might not be stable over time.

Thanks a lot for raising this issue @tobiasstadler !

@SylvainJuge SylvainJuge merged commit 0465d47 into elastic:master Apr 12, 2021
@tobiasstadler
Copy link
Contributor Author

Thank you!

@tobiasstadler tobiasstadler deleted the fix-1735 branch April 13, 2021 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants