Skip to content

Migrate OtelContext wrapper to new Context API - #8530

Merged
mcculls merged 2 commits into
project/contextfrom
mcculls/use-context-for-otel-shim
Mar 10, 2025
Merged

Migrate OtelContext wrapper to new Context API#8530
mcculls merged 2 commits into
project/contextfrom
mcculls/use-context-for-otel-shim

Conversation

@mcculls

@mccullsmcculls commented Mar 9, 2025

Copy link
Copy Markdown
Contributor

Additional Notes

Tests are expected to fail while the scope manager has not yet been replaced.

That's why the final target of this stacked PR is the project/context feature branch.

Contributor Checklist

Jira ticket: APMAPI-855

@mccullsmcculls added tag: no release notes Changes to exclude from release notes type: refactoring inst: opentelemetry OpenTelemetry instrumentation labels Mar 9, 2025
@pr-commenter

pr-commenterBot commented Mar 9, 2025

Copy link
Copy Markdown

Benchmarks

Startup

Load

Parameters

BaselineCandidate
baseline_or_candidatebaselinecandidate
end_time2025-03-10T15:21:542025-03-10T15:29:41
git_branchmastermcculls/use-context-for-otel-shim
git_commit_date17416045041741619390
git_commit_shaa464b9b8305d62
release_version1.48.0-SNAPSHOT~a464b9bfb91.48.0-SNAPSHOT~8305d628cb
start_time2025-03-10T15:21:402025-03-10T15:29:27
See matching parameters
BaselineCandidate
applicationinsecure-bankinsecure-bank
ci_job_date17416209821741620982
ci_job_id840331668840331668
ci_pipeline_id5828559158285591
cpu_modelIntel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHzIntel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
kernel_versionLinux runner-cq9dgfmp-project-304-concurrent-0-w0gj4rg4 6.8.0-1023-aws #25~22.04.1-Ubuntu SMP Tue Jan 28 12:51:22 UTC 2025 x86_64 x86_64 x86_64 GNU/LinuxLinux runner-cq9dgfmp-project-304-concurrent-0-w0gj4rg4 6.8.0-1023-aws #25~22.04.1-Ubuntu SMP Tue Jan 28 12:51:22 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
variantiastiast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 14 metrics, 16 unstable metrics.

Request duration reports for petclinic
gantt
title petclinic - request duration [CI 0.99] : candidate=1.48.0-SNAPSHOT~8305d628cb, baseline=1.48.0-SNAPSHOT~a464b9bfb9
dateFormat X
axisFormat %s
section baseline
no_agent (1.378 ms) : 1358, 1398
. : milestone, 1378,
appsec (1.733 ms) : 1709, 1757
. : milestone, 1733,
appsec_no_iast (1.769 ms) : 1745, 1793
. : milestone, 1769,
code_origins (1.687 ms) : 1654, 1720
. : milestone, 1687,
iast (1.52 ms) : 1495, 1544
. : milestone, 1520,
profiling (1.533 ms) : 1510, 1556
. : milestone, 1533,
tracing (1.486 ms) : 1460, 1512
. : milestone, 1486,
section candidate
no_agent (1.368 ms) : 1348, 1388
. : milestone, 1368,
appsec (1.738 ms) : 1713, 1762
. : milestone, 1738,
appsec_no_iast (1.753 ms) : 1729, 1777
. : milestone, 1753,
code_origins (1.688 ms) : 1656, 1721
. : milestone, 1688,
iast (1.518 ms) : 1493, 1542
. : milestone, 1518,
profiling (1.563 ms) : 1538, 1588
. : milestone, 1563,
tracing (1.502 ms) : 1477, 1526
. : milestone, 1502,
Loading
  • baseline results
VariantRequest duration [CI 0.99]Δ no_agent
no_agent1.378 ms [1.358 ms, 1.398 ms]-
appsec1.733 ms [1.709 ms, 1.757 ms]354.968 µs (25.8%)
appsec_no_iast1.769 ms [1.745 ms, 1.793 ms]390.936 µs (28.4%)
code_origins1.687 ms [1.654 ms, 1.72 ms]309.038 µs (22.4%)
iast1.52 ms [1.495 ms, 1.544 ms]141.696 µs (10.3%)
profiling1.533 ms [1.51 ms, 1.556 ms]155.432 µs (11.3%)
tracing1.486 ms [1.46 ms, 1.512 ms]107.916 µs (7.8%)
  • candidate results
VariantRequest duration [CI 0.99]Δ no_agent
no_agent1.368 ms [1.348 ms, 1.388 ms]-
appsec1.738 ms [1.713 ms, 1.762 ms]369.428 µs (27.0%)
appsec_no_iast1.753 ms [1.729 ms, 1.777 ms]384.943 µs (28.1%)
code_origins1.688 ms [1.656 ms, 1.721 ms]320.207 µs (23.4%)
iast1.518 ms [1.493 ms, 1.542 ms]149.305 µs (10.9%)
profiling1.563 ms [1.538 ms, 1.588 ms]194.87 µs (14.2%)
tracing1.502 ms [1.477 ms, 1.526 ms]133.282 µs (9.7%)
Request duration reports for insecure-bank
gantt
title insecure-bank - request duration [CI 0.99] : candidate=1.48.0-SNAPSHOT~8305d628cb, baseline=1.48.0-SNAPSHOT~a464b9bfb9
dateFormat X
axisFormat %s
section baseline
no_agent (386.367 µs) : 367, 406
. : milestone, 386,
iast (519.741 µs) : 498, 541
. : milestone, 520,
iast_FULL (738.583 µs) : 717, 761
. : milestone, 739,
iast_GLOBAL (567.537 µs) : 545, 590
. : milestone, 568,
iast_HARDCODED_SECRET_DISABLED (520.411 µs) : 499, 542
. : milestone, 520,
iast_INACTIVE (471.656 µs) : 450, 493
. : milestone, 472,
iast_TELEMETRY_OFF (512.497 µs) : 490, 535
. : milestone, 512,
tracing (467.317 µs) : 446, 489
. : milestone, 467,
section candidate
no_agent (387.373 µs) : 367, 407
. : milestone, 387,
iast (522.817 µs) : 500, 546
. : milestone, 523,
iast_FULL (735.848 µs) : 714, 758
. : milestone, 736,
iast_GLOBAL (565.841 µs) : 543, 588
. : milestone, 566,
iast_HARDCODED_SECRET_DISABLED (518.734 µs) : 497, 540
. : milestone, 519,
iast_INACTIVE (469.796 µs) : 449, 491
. : milestone, 470,
iast_TELEMETRY_OFF (501.849 µs) : 480, 523
. : milestone, 502,
tracing (471.13 µs) : 450, 492
. : milestone, 471,
Loading
  • baseline results
VariantRequest duration [CI 0.99]Δ no_agent
no_agent386.367 µs [366.769 µs, 405.965 µs]-
iast519.741 µs [498.152 µs, 541.33 µs]133.374 µs (34.5%)
iast_FULL738.583 µs [716.657 µs, 760.509 µs]352.216 µs (91.2%)
iast_GLOBAL567.537 µs [545.355 µs, 589.718 µs]181.17 µs (46.9%)
iast_HARDCODED_SECRET_DISABLED520.411 µs [498.808 µs, 542.015 µs]134.044 µs (34.7%)
iast_INACTIVE471.656 µs [449.926 µs, 493.386 µs]85.289 µs (22.1%)
iast_TELEMETRY_OFF512.497 µs [489.603 µs, 535.39 µs]126.13 µs (32.6%)
tracing467.317 µs [445.952 µs, 488.682 µs]80.95 µs (21.0%)
  • candidate results
VariantRequest duration [CI 0.99]Δ no_agent
no_agent387.373 µs [367.48 µs, 407.265 µs]-
iast522.817 µs [499.761 µs, 545.872 µs]135.444 µs (35.0%)
iast_FULL735.848 µs [713.986 µs, 757.711 µs]348.476 µs (90.0%)
iast_GLOBAL565.841 µs [543.254 µs, 588.428 µs]178.468 µs (46.1%)
iast_HARDCODED_SECRET_DISABLED518.734 µs [497.408 µs, 540.06 µs]131.361 µs (33.9%)
iast_INACTIVE469.796 µs [448.698 µs, 490.893 µs]82.423 µs (21.3%)
iast_TELEMETRY_OFF501.849 µs [480.25 µs, 523.448 µs]114.476 µs (29.6%)
tracing471.13 µs [450.158 µs, 492.102 µs]83.757 µs (21.6%)

Dacapo

Parameters

BaselineCandidate
baseline_or_candidatebaselinecandidate
git_branchmastermcculls/use-context-for-otel-shim
git_commit_date17416045041741619390
git_commit_shaa464b9b8305d62
release_version1.48.0-SNAPSHOT~a464b9bfb91.48.0-SNAPSHOT~8305d628cb
See matching parameters
BaselineCandidate
applicationbiojavabiojava
ci_job_date17416214541741621454
ci_job_id840331673840331673
ci_pipeline_id5828559158285591
cpu_modelIntel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHzIntel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
kernel_versionLinux runner-5uqttet-project-304-concurrent-0-jp6o4qqo 6.8.0-1023-aws #25~22.04.1-Ubuntu SMP Tue Jan 28 12:51:22 UTC 2025 x86_64 x86_64 x86_64 GNU/LinuxLinux runner-5uqttet-project-304-concurrent-0-jp6o4qqo 6.8.0-1023-aws #25~22.04.1-Ubuntu SMP Tue Jan 28 12:51:22 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
variantappsecappsec

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics.

Execution time for biojava
gantt
title biojava - execution time [CI 0.99] : candidate=1.48.0-SNAPSHOT~8305d628cb, baseline=1.48.0-SNAPSHOT~a464b9bfb9
dateFormat X
axisFormat %s
section baseline
no_agent (15.082 s) : 15082000, 15082000
. : milestone, 15082000,
appsec (14.926 s) : 14926000, 14926000
. : milestone, 14926000,
iast (18.98 s) : 18980000, 18980000
. : milestone, 18980000,
iast_GLOBAL (17.936 s) : 17936000, 17936000
. : milestone, 17936000,
profiling (15.665 s) : 15665000, 15665000
. : milestone, 15665000,
tracing (14.852 s) : 14852000, 14852000
. : milestone, 14852000,
section candidate
no_agent (15.364 s) : 15364000, 15364000
. : milestone, 15364000,
appsec (15.177 s) : 15177000, 15177000
. : milestone, 15177000,
iast (18.355 s) : 18355000, 18355000
. : milestone, 18355000,
iast_GLOBAL (17.805 s) : 17805000, 17805000
. : milestone, 17805000,
profiling (15.66 s) : 15660000, 15660000
. : milestone, 15660000,
tracing (15.066 s) : 15066000, 15066000
. : milestone, 15066000,
Loading
  • baseline results
VariantExecution Time [CI 0.99]Δ no_agent
no_agent15.082 s [15.082 s, 15.082 s]-
appsec14.926 s [14.926 s, 14.926 s]-156.0 ms (-1.0%)
iast18.98 s [18.98 s, 18.98 s]3.898 s (25.8%)
iast_GLOBAL17.936 s [17.936 s, 17.936 s]2.854 s (18.9%)
profiling15.665 s [15.665 s, 15.665 s]583.0 ms (3.9%)
tracing14.852 s [14.852 s, 14.852 s]-230.0 ms (-1.5%)
  • candidate results
VariantExecution Time [CI 0.99]Δ no_agent
no_agent15.364 s [15.364 s, 15.364 s]-
appsec15.177 s [15.177 s, 15.177 s]-187.0 ms (-1.2%)
iast18.355 s [18.355 s, 18.355 s]2.991 s (19.5%)
iast_GLOBAL17.805 s [17.805 s, 17.805 s]2.441 s (15.9%)
profiling15.66 s [15.66 s, 15.66 s]296.0 ms (1.9%)
tracing15.066 s [15.066 s, 15.066 s]-298.0 ms (-1.9%)
Execution time for tomcat
gantt
title tomcat - execution time [CI 0.99] : candidate=1.48.0-SNAPSHOT~8305d628cb, baseline=1.48.0-SNAPSHOT~a464b9bfb9
dateFormat X
axisFormat %s
section baseline
no_agent (1.47 ms) : 1459, 1482
. : milestone, 1470,
appsec (2.325 ms) : 2281, 2369
. : milestone, 2325,
iast (2.113 ms) : 2057, 2169
. : milestone, 2113,
iast_GLOBAL (2.159 ms) : 2103, 2215
. : milestone, 2159,
profiling (1.964 ms) : 1920, 2008
. : milestone, 1964,
tracing (1.953 ms) : 1910, 1996
. : milestone, 1953,
section candidate
no_agent (1.474 ms) : 1463, 1486
. : milestone, 1474,
appsec (2.328 ms) : 2285, 2372
. : milestone, 2328,
iast (2.105 ms) : 2050, 2159
. : milestone, 2105,
iast_GLOBAL (2.154 ms) : 2099, 2210
. : milestone, 2154,
profiling (1.96 ms) : 1917, 2003
. : milestone, 1960,
tracing (1.949 ms) : 1907, 1992
. : milestone, 1949,
Loading
  • baseline results
VariantExecution Time [CI 0.99]Δ no_agent
no_agent1.47 ms [1.459 ms, 1.482 ms]-
appsec2.325 ms [2.281 ms, 2.369 ms]854.703 µs (58.1%)
iast2.113 ms [2.057 ms, 2.169 ms]642.904 µs (43.7%)
iast_GLOBAL2.159 ms [2.103 ms, 2.215 ms]688.925 µs (46.9%)
profiling1.964 ms [1.92 ms, 2.008 ms]493.912 µs (33.6%)
tracing1.953 ms [1.91 ms, 1.996 ms]482.63 µs (32.8%)
  • candidate results
VariantExecution Time [CI 0.99]Δ no_agent
no_agent1.474 ms [1.463 ms, 1.486 ms]-
appsec2.328 ms [2.285 ms, 2.372 ms]854.108 µs (57.9%)
iast2.105 ms [2.05 ms, 2.159 ms]630.568 µs (42.8%)
iast_GLOBAL2.154 ms [2.099 ms, 2.21 ms]680.319 µs (46.2%)
profiling1.96 ms [1.917 ms, 2.003 ms]485.798 µs (33.0%)
tracing1.949 ms [1.907 ms, 1.992 ms]475.048 µs (32.2%)

@PerfectSlayerPerfectSlayer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I left few minor comments.

About the test changes, are they put in place because context tracking is missing and resources are leaking as the tests can run to the end? Because the cleanup seem to be already part of the test steps.
If so, should we really merge them as it will get confusing once the tests are working again properly?

@mcculls
mccullsforce-pushed the mcculls/remove-scopestateaware-api branch from 5e870d7 to 7724a95CompareMarch 10, 2025 13:05
Base automatically changed from mcculls/remove-scopestateaware-api to project/contextMarch 10, 2025 13:06
@mcculls
mccullsforce-pushed the mcculls/use-context-for-otel-shim branch from 5c793f8 to 591efc8CompareMarch 10, 2025 13:06
@mcculls

mcculls commented Mar 10, 2025

Copy link
Copy Markdown
ContributorAuthor

About the test changes, are they put in place because context tracking is missing and resources are leaking as the tests can run to the end? Because the cleanup seem to be already part of the test steps.

So the test changes here are adding missing cleanup of scopes/spans that's done for other tests in the class but was missed here. These tests will fail at the moment because we haven't finished the span <-> context coupling, so while OtelContext is now context-based it needs the scope manager to be fully replaced in order for the underlying context to reflect spans being activated, etc. using our internal API.

However there's a difference between a test failing (which helps us focus on what remains to be fixed) and it causing unrelated failures in other tests (which would pass without the bad scope state leaking from the failing tests.)

So these are really test fixes, to ensure failing tests don't cause unrelated tests to fail.

( in other words, we'd still want to keep them because they make sure test failures don't leak to the other tests )

@mcculls
mcculls marked this pull request as ready for review March 10, 2025 15:20
@mcculls
mcculls requested review from a team as code ownersMarch 10, 2025 15:20
@mcculls
mcculls requested review from dougqh and removed request for dougqhMarch 10, 2025 15:21
@mcculls
mcculls merged commit 819daaa into project/contextMar 10, 2025
@mcculls
mcculls deleted the mcculls/use-context-for-otel-shim branch March 10, 2025 16:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inst: opentelemetryOpenTelemetry instrumentationtag: no release notesChanges to exclude from release notestype: refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mcculls@PerfectSlayer