Skip to content

Suppress segments in HttpUrlConnection #2365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 20, 2025
Merged

Conversation

kanderson250
Copy link
Contributor

Resolves #2337

Our HttpUrlConnection instrumentation creates a tracer for each response handler called by the HttpUrlConnection instance. We do this to ensure we have some tracer available to report the external call. This isn't ideal, because only the first tracer gets used to report the external, but it is not known at the time each tracer is created (via @Trace annotation) whether it is the first one. The result is that HttpUrlConnection transaction traces/DTs may be cluttered with a great number of response handler segments (such as getInputStream) that weren't chosen to become externals.

This PR introduces a low-verbose mode to give users the option of suppressing these non-external segments in transaction traces and DTs. This can be enabled by setting:

NEW_RELIC_CLASS_TRANSFORMER_COM_NEWRELIC_INSTRUMENTATION_HTTURLCONNECTION_VERBOSE=false, sys prop -Dnewrelic.config.class_transformer.com.newrelic.instrumentation.httpurlconnection.verbose=false, or equivalent stanza in newrelic.yml. Default setting is true (ie, non-external getInputStream and other response handler methods will be reported as before).

Implementation comments

This functionality is implemented via an internal-only API method, excludeLeaf, which has the ability to toggle the excludes tracer flag on leaf tracers, just like the annotation @Trace(excludeFromTransactionTrace=true). The difference is that the new excludeLeaf method can be called after the tracer has already been created, whereas the annotation applies only when the tracer is constructed.

excludeLeaf is a no-op everywhere but in DefaultTracer. It is also a no-op in root tracer implementations to stay consistent with our existing agent behavior that prohibits root tracers from being excluded.

We chose to make excludeLeaf (rather than a general excludeTracer) to ensure we don't unintentionally exclude parent tracers and orphan their children. However, this could be revisited in the future as a potential enhancement.

Tests

There are a few new unit tests, plus a small AIT update for the HttpUrlConnection instrumentation. AIT branch is here and test run for that against this PR is here.

@codecov-commenter
Copy link

codecov-commenter commented May 19, 2025

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.52%. Comparing base (a0d08e3) to head (8fcb263).
Report is 58 commits behind head on main.

Files with missing lines Patch % Lines
...ava/com/newrelic/agent/tracers/AbstractTracer.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2365      +/-   ##
============================================
- Coverage     70.58%   70.52%   -0.06%     
+ Complexity    10042    10034       -8     
============================================
  Files           844      844              
  Lines         40750    40635     -115     
  Branches       6191     6161      -30     
============================================
- Hits          28762    28658     -104     
+ Misses         9210     9192      -18     
- Partials       2778     2785       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kanderson250 kanderson250 merged commit 2a4e5b9 into main May 20, 2025
114 checks passed
@github-project-automation github-project-automation bot moved this from Triage to Code Complete/Done in Java Engineering Board May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[esc] Explore options to disable the getInputStream spans
3 participants