inspectIT / inspectit-ocelot

inspectIT Ocelot - Java agent for collecting application performance, tracing and behavior data
http://www.inspectit.rocks/
Apache License 2.0
204 stars 69 forks source link

Closes #1555: [Feature] - fix StackTraceSampler / AutoTracing #1558

Closed heiko-holz closed 1 year ago

heiko-holz commented 1 year ago

Closes #1555


This change is Reviewable

codecov[bot] commented 1 year ago

Codecov Report

Merging #1558 (eb2637d) into master (e1d0861) will increase coverage by 0.70%. The diff coverage is 58.61%.

:exclamation: Current head eb2637d differs from pull request most recent head 2d4b6cc. Consider uploading reports for the commit 2d4b6cc to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1558 +/- ## ============================================ + Coverage 77.54% 78.24% +0.70% - Complexity 2320 2404 +84 ============================================ Files 240 246 +6 Lines 7730 7909 +179 Branches 924 936 +12 ============================================ + Hits 5994 6188 +194 + Misses 1350 1307 -43 - Partials 386 414 +28 ``` | [Impacted Files](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT) | Coverage Δ | | |---|---|---| | [...tstrap/opentelemetry/IOpenTelemetryController.java](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT#diff-aW5zcGVjdGl0LW9jZWxvdC1ib290c3RyYXAvc3JjL21haW4vamF2YS9yb2Nrcy9pbnNwZWN0aXQvb2NlbG90L2Jvb3RzdHJhcC9vcGVudGVsZW1ldHJ5L0lPcGVuVGVsZW1ldHJ5Q29udHJvbGxlci5qYXZh) | `100.00% <ø> (ø)` | | | [...rap/opentelemetry/NoopOpenTelemetryController.java](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT#diff-aW5zcGVjdGl0LW9jZWxvdC1ib290c3RyYXAvc3JjL21haW4vamF2YS9yb2Nrcy9pbnNwZWN0aXQvb2NlbG90L2Jvb3RzdHJhcC9vcGVudGVsZW1ldHJ5L05vb3BPcGVuVGVsZW1ldHJ5Q29udHJvbGxlci5qYXZh) | `20.00% <0.00%> (-2.22%)` | :arrow_down: | | [...del/instrumentation/rules/RuleTracingSettings.java](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT#diff-aW5zcGVjdGl0LW9jZWxvdC1jb25maWcvc3JjL21haW4vamF2YS9yb2Nrcy9pbnNwZWN0aXQvb2NlbG90L2NvbmZpZy9tb2RlbC9pbnN0cnVtZW50YXRpb24vcnVsZXMvUnVsZVRyYWNpbmdTZXR0aW5ncy5qYXZh) | `100.00% <ø> (ø)` | | | [...t/ocelot/config/model/tracing/TracingSettings.java](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT#diff-aW5zcGVjdGl0LW9jZWxvdC1jb25maWcvc3JjL21haW4vamF2YS9yb2Nrcy9pbnNwZWN0aXQvb2NlbG90L2NvbmZpZy9tb2RlbC90cmFjaW5nL1RyYWNpbmdTZXR0aW5ncy5qYXZh) | `100.00% <ø> (ø)` | | | [...t/core/instrumentation/autotracing/Invocation.java](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT#diff-aW5zcGVjdGl0LW9jZWxvdC1jb3JlL3NyYy9tYWluL2phdmEvcm9ja3MvaW5zcGVjdGl0L29jZWxvdC9jb3JlL2luc3RydW1lbnRhdGlvbi9hdXRvdHJhY2luZy9JbnZvY2F0aW9uLmphdmE=) | `90.91% <ø> (ø)` | | | [...nstrumentation/autotracing/InvocationResolver.java](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT#diff-aW5zcGVjdGl0LW9jZWxvdC1jb3JlL3NyYy9tYWluL2phdmEvcm9ja3MvaW5zcGVjdGl0L29jZWxvdC9jb3JlL2luc3RydW1lbnRhdGlvbi9hdXRvdHJhY2luZy9JbnZvY2F0aW9uUmVzb2x2ZXIuamF2YQ==) | `89.66% <0.00%> (+1.42%)` | :arrow_up: | | [...e/instrumentation/autotracing/PlaceholderSpan.java](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT#diff-aW5zcGVjdGl0LW9jZWxvdC1jb3JlL3NyYy9tYWluL2phdmEvcm9ja3MvaW5zcGVjdGl0L29jZWxvdC9jb3JlL2luc3RydW1lbnRhdGlvbi9hdXRvdHJhY2luZy9QbGFjZWhvbGRlclNwYW4uamF2YQ==) | `0.00% <0.00%> (-2.44%)` | :arrow_down: | | [...mentation/autotracing/events/MethodEntryEvent.java](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT#diff-aW5zcGVjdGl0LW9jZWxvdC1jb3JlL3NyYy9tYWluL2phdmEvcm9ja3MvaW5zcGVjdGl0L29jZWxvdC9jb3JlL2luc3RydW1lbnRhdGlvbi9hdXRvdHJhY2luZy9ldmVudHMvTWV0aG9kRW50cnlFdmVudC5qYXZh) | `100.00% <ø> (ø)` | | | [...instrumentation/autotracing/events/TraceEvent.java](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT#diff-aW5zcGVjdGl0LW9jZWxvdC1jb3JlL3NyYy9tYWluL2phdmEvcm9ja3MvaW5zcGVjdGl0L29jZWxvdC9jb3JlL2luc3RydW1lbnRhdGlvbi9hdXRvdHJhY2luZy9ldmVudHMvVHJhY2VFdmVudC5qYXZh) | `100.00% <ø> (ø)` | | | [...nstrumentation/hook/actions/TracingHookAction.java](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT#diff-aW5zcGVjdGl0LW9jZWxvdC1jb3JlL3NyYy9tYWluL2phdmEvcm9ja3MvaW5zcGVjdGl0L29jZWxvdC9jb3JlL2luc3RydW1lbnRhdGlvbi9ob29rL2FjdGlvbnMvVHJhY2luZ0hvb2tBY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | | | ... and [41 more](https://codecov.io/gh/inspectIT/inspectit-ocelot/pull/1558?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inspectIT) | |
heiko-holz commented 1 year ago

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/opentelemetry/trace/samplers/HybridParentTraceIdRatioBasedSampler.java line 52 at r1 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…
Technically we could. However, I have followed OpenCensus' implementation of https://github.com/census-instrumentation/opencensus-java/blob/master/api/src/main/java/io/opencensus/trace/samplers/ProbabilitySampler.java. But we can change it to this if you think this is more readable: ```Java // If the parent or any parent link has been sampled keep, the sampling decision. if ((parentSpanContext.isValid() && parentSpanContext.isSampled()) || isAnyParentLinkSampled(parentLinks)) { return SamplingResult.recordAndSample(); } ```

We have agreed to change it to the shorter version