Skip to content

Internal - address code scanning and removed dead code#462

Merged
cleverchuk merged 3 commits into
mainfrom
cc/NH-133281
Apr 15, 2026
Merged

Internal - address code scanning and removed dead code#462
cleverchuk merged 3 commits into
mainfrom
cc/NH-133281

Conversation

@cleverchuk
Copy link
Copy Markdown
Contributor

Summary

Remove dead code from the Context class and address code scanning findings across core and sampling modules. The Context class no longer maintains its own thread-local metadata state — it now delegates entirely to the current OpenTelemetry SpanContext. Code profiling smoke tests are re-enabled with a new dedicated verification approach.

Dead Code Removal — Context class

  • Removed the InheritableThreadLocal<Metadata> and all associated thread-local context management (setMetadata, clearMetadata, setSkipInheritingContext, skipInheritingContextThreadLocal)
  • Removed the SettingsArg.DISABLE_INHERIT_CONTEXT listener that toggled context inheritance behavior
  • Removed unused factory methods: createEvent(), createEventWithID(), createEventWithIDAndContext(), createEventWithGeneratedMetadata()
  • getMetadata() now simply constructs a Metadata from the current OTel Span.current().getSpanContext() instead of reading from thread-local storage
  • Removed the Event.report(EventReporter) single-arg overload (and its implementations in EventImpl and NoopEvent) — callers now always pass the Metadata explicitly

Code Scanning Fixes

  • Metadata(SpanContext) constructor: removed the isValid() guard — the caller is responsible for validity; this avoids leaving fields in an inconsistent half-initialized state
  • TimeUtils.getSum(): replaced autoboxing of primitive Doubledouble and explicit Double.valueOf() to avoid unnecessary boxing flagged by code scanning
  • SolarwindsProfilingSpanProcessor: inlined a single-use local variable (spanContext) to reduce scope
  • ServerHostInfoReader.getHostId(): removed the Context.clearMetadata()/setMetadata() save-restore pattern that was guarding against accidental tracing during init — no longer needed since Context doesn't hold mutable state
  • Removed unused getDockerContainerId() method and HOSTNAME_SEPARATOR constant from ServerHostInfoReader
  • Replaced raw type diamond operators (new HashMap<String, String>()new HashMap<>()) and System.getProperty("line.separator")System.lineSeparator()
  • Cleaned up empty @param/@return javadoc tags

SettingsArg Rename

  • Renamed the readValue(ByteBuffer) method to readFromByteBuffer(ByteBuffer) across all SettingsArg subtypes to disambiguate it from the readValue(Object) overload

Observability Additions

  • Added @ToString (Lombok) to SamplingConfiguration and TraceConfigs for better logging/debugging visibility

Smoke Test — Code Profiling

  • Re-enabled the assertCodeProfiling test (removed @Disabled) in both SmokeTest and SmokeTestV2
  • Added a new verify_profile function in the k6 script that queries the SWO GraphQL API for spans with sw.profile.spans:1, replacing the previous inline property check (NewFrames key)
  • Enabled the profiler in the smoke test agent config (-Dsw.apm.profiler.enabled=true, -Dsw.apm.profiler.interval=10)

Test Updates

  • All tests updated to use explicit Metadata instances and createEventWithContext(metadata) / report(metadata, reporter) instead of the removed Context.createEvent() / event.report(reporter) patterns
  • Removed testInheritContext test and TestThread helper class since thread-local context inheritance no longer exists
  • Removed startTrace() helper methods from EventImplTest and TestReporterTest — replaced with inline metadata setup

Test services data

  1. e-1712644058766987264
  2. e-1712643928659124224
  3. e-1742334541200846848
  4. e-1777406072376840192

Copilot AI review requested due to automatic review settings April 13, 2026 18:45
@cleverchuk cleverchuk requested review from a team as code owners April 13, 2026 18:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes legacy thread-local context handling and dead code in the core Context/Event flow, shifting metadata derivation to the active OpenTelemetry SpanContext, while also addressing code-scanning findings and re-enabling profiling smoke-test verification.

Changes:

  • Simplified Context to derive Metadata from Span.current().getSpanContext() and removed deprecated context mutation / dead factory methods.
  • Updated callers/tests to pass Metadata explicitly into event creation/reporting and adjusted settings parsing API (readFromByteBuffer rename).
  • Re-enabled profiling smoke tests by enabling the profiler in agent config and adding a new k6-based GraphQL verification path.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
smoke-tests/src/test/java/com/solarwinds/agents/Agent.java Enables profiler flags for smoke-test agents.
smoke-tests/src/test/java/com/solarwinds/SmokeTestV2.java Re-enables code profiling assertion.
smoke-tests/src/test/java/com/solarwinds/SmokeTest.java Re-enables code profiling assertion.
smoke-tests/k6/basic.js Adds new GraphQL-based profiling verification and removes old NewFrames property check.
libs/sampling/src/test/java/com/solarwinds/joboe/sampling/SettingsArgTest.java Updates tests for renamed settings parsing API.
libs/sampling/src/main/java/com/solarwinds/joboe/sampling/TraceConfigs.java Adds toString() for improved diagnostics.
libs/sampling/src/main/java/com/solarwinds/joboe/sampling/SettingsArg.java Renames readValue(ByteBuffer) to readFromByteBuffer(ByteBuffer).
libs/sampling/src/main/java/com/solarwinds/joboe/sampling/SamplingConfiguration.java Adds toString() for improved diagnostics.
libs/sampling/src/main/java/com/solarwinds/joboe/sampling/Metadata.java Makes Metadata(SpanContext) fully initialize fields without isValid() gating.
libs/core/src/test/java/com/solarwinds/joboe/core/rpc/RpcClientTest.java Updates tests to explicitly construct/pass Metadata into event creation.
libs/core/src/test/java/com/solarwinds/joboe/core/TestReporterTest.java Removes thread-local setup helper and reports with explicit Metadata.
libs/core/src/test/java/com/solarwinds/joboe/core/EventImplTest.java Updates tests for explicit Metadata usage and removes thread-local cleanup.
libs/core/src/test/java/com/solarwinds/joboe/core/ContextTest.java Removes inheritance/thread-local tests and updates for explicit Metadata usage.
libs/core/src/main/java/com/solarwinds/joboe/core/util/TimeUtils.java Code-scanning-driven change to getSum() boxing behavior.
libs/core/src/main/java/com/solarwinds/joboe/core/util/ServerHostInfoReader.java Removes obsolete context save/restore and dead code; modernizes collections/line separator usage.
libs/core/src/main/java/com/solarwinds/joboe/core/rpc/RpcSettings.java Updates settings argument parsing to use readFromByteBuffer.
libs/core/src/main/java/com/solarwinds/joboe/core/NoopEvent.java Removes obsolete report(EventReporter) overload implementation.
libs/core/src/main/java/com/solarwinds/joboe/core/EventImpl.java Removes obsolete report(EventReporter) overload.
libs/core/src/main/java/com/solarwinds/joboe/core/Event.java Removes obsolete report(EventReporter) overload from API.
libs/core/src/main/java/com/solarwinds/joboe/core/Context.java Removes thread-local context state; derives Metadata from OTel SpanContext.
custom/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsProfilingSpanProcessor.java Minor refactor (inline single-use local) in profiling span processor.

Comment thread smoke-tests/k6/basic.js
Comment thread libs/core/src/main/java/com/solarwinds/joboe/core/util/TimeUtils.java Outdated
Comment thread smoke-tests/k6/basic.js
Comment thread smoke-tests/k6/basic.js Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…event ClassCastException

Agent-Logs-Url: https://github.com/solarwinds/apm-java/sessions/f03ea42c-8a1f-4a6a-bb03-e05afe58844b

Co-authored-by: cleverchuk <15526124+cleverchuk@users.noreply.github.com>
Copy link
Copy Markdown

@jerrytfleung jerrytfleung left a comment

Choose a reason for hiding this comment

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

LGTM. I like the explicit metadata in report function that making more sense on metadata handling.

@cleverchuk cleverchuk merged commit 7115a89 into main Apr 15, 2026
16 checks passed
@cleverchuk cleverchuk deleted the cc/NH-133281 branch April 15, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants