Skip to content
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

Standardize common configuration telemetry #6694

Merged
merged 9 commits into from Feb 23, 2024

Conversation

ygree
Copy link
Contributor

@ygree ygree commented Feb 17, 2024

What Does This Do

  • Collects null config defaults
  • Serializes maps as string, empty maps as empty strings
  • Standardizes common settings naming
    • Converts to snake case when reported with the telemetry
    • Introduces logs_injection_enabled with the logs_injection alias
    • Introduces trace_tags with the tags alias
  • Standardize common settings values?
    • All setting values converted to strings except for null
      • Empty maps and lists reported as empty strings
    • Defaults the sampling rate to null (JUST AS IT WAS), which means sample everything, but is not exactly the same as if it's explicitly set to 1.0
    • Allows reordering of map setting keys for some map settings (JUST AS IT WAS)
    • Allows lowercase keys for some merged map settings from different sources (JUST AS IT WAS)

Motivation

In-app APM Library Configuration Reporting

Additional Notes

Jira ticket: APMJAVA-1217

Corresponding Parametric Tests
Similar Python Tracer change

@ygree ygree self-assigned this Feb 17, 2024
@pr-commenter
Copy link

pr-commenter bot commented Feb 17, 2024

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master ygree/telemetry-config-standardization
git_commit_date 1708720757 1708723831
git_commit_sha 9032a67 4f7daf4
release_version 1.31.0-SNAPSHOT~9032a67633 1.31.0-SNAPSHOT~4f7daf4012
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1708726813 1708726813
ci_job_id 442350303 442350303
ci_pipeline_id 28851706 28851706
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
module Agent Agent
parent None None
variant iast iast

Summary

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

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-02-23T21:56:36 2024-02-23T22:15:12
git_branch master ygree/telemetry-config-standardization
git_commit_date 1708720757 1708723831
git_commit_sha 9032a67 4f7daf4
release_version 1.31.0-SNAPSHOT~9032a67633 1.31.0-SNAPSHOT~4f7daf4012
start_time 2024-02-23T21:56:22 2024-02-23T22:14:59
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1708726813 1708726813
ci_job_id 442350303 442350303
ci_pipeline_id 28851706 28851706
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant iast iast

Summary

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

Request duration reports for insecure-bank
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.31.0-SNAPSHOT~4f7daf4012, baseline=1.31.0-SNAPSHOT~9032a67633
    dateFormat X
    axisFormat %s
section baseline
no_agent (371.04 µs) : 351, 391
.   : milestone, 371,
iast (483.464 µs) : 462, 505
.   : milestone, 483,
iast_FULL (546.128 µs) : 525, 567
.   : milestone, 546,
iast_GLOBAL (500.437 µs) : 480, 521
.   : milestone, 500,
iast_HARDCODED_SECRET_DISABLED (482.121 µs) : 462, 503
.   : milestone, 482,
iast_INACTIVE (457.646 µs) : 437, 479
.   : milestone, 458,
iast_TELEMETRY_OFF (473.074 µs) : 453, 494
.   : milestone, 473,
tracing (446.454 µs) : 426, 467
.   : milestone, 446,
section candidate
no_agent (367.797 µs) : 347, 389
.   : milestone, 368,
iast (478.477 µs) : 458, 499
.   : milestone, 478,
iast_FULL (538.188 µs) : 518, 559
.   : milestone, 538,
iast_GLOBAL (491.355 µs) : 471, 512
.   : milestone, 491,
iast_HARDCODED_SECRET_DISABLED (478.25 µs) : 458, 499
.   : milestone, 478,
iast_INACTIVE (449.64 µs) : 428, 471
.   : milestone, 450,
iast_TELEMETRY_OFF (469.648 µs) : 449, 490
.   : milestone, 470,
tracing (457.915 µs) : 438, 478
.   : milestone, 458,
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 371.04 µs [351.389 µs, 390.69 µs] -
iast 483.464 µs [462.201 µs, 504.727 µs] 112.425 µs (30.3%)
iast_FULL 546.128 µs [525.439 µs, 566.816 µs] 175.088 µs (47.2%)
iast_GLOBAL 500.437 µs [479.609 µs, 521.265 µs] 129.398 µs (34.9%)
iast_HARDCODED_SECRET_DISABLED 482.121 µs [461.537 µs, 502.706 µs] 111.082 µs (29.9%)
iast_INACTIVE 457.646 µs [436.678 µs, 478.615 µs] 86.607 µs (23.3%)
iast_TELEMETRY_OFF 473.074 µs [452.535 µs, 493.613 µs] 102.035 µs (27.5%)
tracing 446.454 µs [425.692 µs, 467.216 µs] 75.414 µs (20.3%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 367.797 µs [346.514 µs, 389.08 µs] -
iast 478.477 µs [458.081 µs, 498.873 µs] 110.68 µs (30.1%)
iast_FULL 538.188 µs [517.765 µs, 558.611 µs] 170.392 µs (46.3%)
iast_GLOBAL 491.355 µs [470.828 µs, 511.883 µs] 123.559 µs (33.6%)
iast_HARDCODED_SECRET_DISABLED 478.25 µs [457.666 µs, 498.835 µs] 110.454 µs (30.0%)
iast_INACTIVE 449.64 µs [428.464 µs, 470.816 µs] 81.843 µs (22.3%)
iast_TELEMETRY_OFF 469.648 µs [449.141 µs, 490.155 µs] 101.851 µs (27.7%)
tracing 457.915 µs [437.517 µs, 478.313 µs] 90.119 µs (24.5%)
Request duration reports for petclinic
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.31.0-SNAPSHOT~4f7daf4012, baseline=1.31.0-SNAPSHOT~9032a67633
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.349 ms) : 1330, 1368
.   : milestone, 1349,
appsec (1.784 ms) : 1761, 1808
.   : milestone, 1784,
iast (1.524 ms) : 1500, 1548
.   : milestone, 1524,
profiling (1.554 ms) : 1529, 1578
.   : milestone, 1554,
tracing (1.492 ms) : 1469, 1516
.   : milestone, 1492,
section candidate
no_agent (1.35 ms) : 1331, 1369
.   : milestone, 1350,
appsec (1.798 ms) : 1775, 1821
.   : milestone, 1798,
iast (1.554 ms) : 1531, 1578
.   : milestone, 1554,
profiling (1.534 ms) : 1510, 1558
.   : milestone, 1534,
tracing (1.496 ms) : 1472, 1520
.   : milestone, 1496,
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.349 ms [1.33 ms, 1.368 ms] -
appsec 1.784 ms [1.761 ms, 1.808 ms] 435.381 µs (32.3%)
iast 1.524 ms [1.5 ms, 1.548 ms] 175.242 µs (13.0%)
profiling 1.554 ms [1.529 ms, 1.578 ms] 204.508 µs (15.2%)
tracing 1.492 ms [1.469 ms, 1.516 ms] 143.334 µs (10.6%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.35 ms [1.331 ms, 1.369 ms] -
appsec 1.798 ms [1.775 ms, 1.821 ms] 447.769 µs (33.2%)
iast 1.554 ms [1.531 ms, 1.578 ms] 204.324 µs (15.1%)
profiling 1.534 ms [1.51 ms, 1.558 ms] 183.601 µs (13.6%)
tracing 1.496 ms [1.472 ms, 1.52 ms] 145.692 µs (10.8%)

@ygree ygree force-pushed the ygree/telemetry-config-standardization branch from 75bbd5a to 3c7799b Compare February 17, 2024 06:20
@ygree ygree changed the title Collect null config defaults. Collect null config defaults Feb 17, 2024
@ygree ygree force-pushed the ygree/telemetry-config-standardization branch 3 times, most recently from b84c2bf to ff12525 Compare February 18, 2024 05:11
@ygree ygree force-pushed the ygree/telemetry-config-standardization branch from ff12525 to 1804a50 Compare February 18, 2024 06:18
@ygree ygree changed the title Collect null config defaults Telemetry Config Standardization Feb 18, 2024
@ygree ygree changed the title Telemetry Config Standardization Common Config Standardization Feb 18, 2024
@ygree ygree marked this pull request as ready for review February 20, 2024 16:17
@ygree ygree requested a review from a team as a code owner February 20, 2024 16:17
@ygree ygree force-pushed the ygree/telemetry-config-standardization branch from 1804a50 to bcb6fde Compare February 21, 2024 04:28
@ygree ygree force-pushed the ygree/telemetry-config-standardization branch from bcb6fde to 0f4353d Compare February 21, 2024 04:38
@PerfectSlayer
Copy link
Contributor

Hey @ygree 👋
According to the PR description, there is still some stuff to standardize.
Is the PR ready to review or should we wait for them first?

@Kyle-Verhoog Kyle-Verhoog self-requested a review February 21, 2024 19:27
@ygree ygree added the comp: telemetry Telemetry label Feb 21, 2024
@ygree
Copy link
Contributor Author

ygree commented Feb 21, 2024

Hey @ygree 👋 According to the PR description, there is still some stuff to standardize. Is the PR ready to review or should we wait for them first?

@PerfectSlayer We have discussed all open questions and I have resolved all TODOs

@mcculls mcculls self-requested a review February 23, 2024 10:05
Copy link
Contributor

@PerfectSlayer PerfectSlayer left a comment

Choose a reason for hiding this comment

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

Looks good to me but you might want another pair of eyes.
I got few minor questions but I am concerned about the default sampling rate change. Is it a breaking change or something worth notifying the customer on the release notes?

@PerfectSlayer PerfectSlayer changed the title Common Config Standardization Standardize common configuration telemetry Feb 23, 2024
@ygree
Copy link
Contributor Author

ygree commented Feb 23, 2024

Looks good to me but you might want another pair of eyes. I got few minor questions but I am concerned about the default sampling rate change. Is it a breaking change or something worth notifying the customer on the release notes?

We end up agreeing to leave the default sample rate untouched. I've changed the PR description to explicitly state this.

@ygree ygree force-pushed the ygree/telemetry-config-standardization branch from bc5b5af to a9c3256 Compare February 23, 2024 20:51
@ygree ygree merged commit 6167d5e into master Feb 23, 2024
79 checks passed
@ygree ygree deleted the ygree/telemetry-config-standardization branch February 23, 2024 22:48
@github-actions github-actions bot added this to the 1.31.0 milestone Feb 23, 2024
@ygree ygree added the run-tests: all Run all tests label Feb 23, 2024
@@ -36,7 +36,7 @@ instrumentation_modules: &instrumentation_modules "dd-java-agent/instrumentation
debugger_modules: &debugger_modules "dd-java-agent/agent-debugger|dd-java-agent/agent-bootstrap|dd-java-agent/agent-builder|internal-api|communication|dd-trace-core"
profiling_modules: &profiling_modules "dd-java-agent/agent-profiling"

default_system_tests_commit: &default_system_tests_commit 2374d120bbbc94109c449a0a9c182a9d4d0af60e
default_system_tests_commit: &default_system_tests_commit 899511453a9beb4dd560eb45ec45a8328383c80a
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Point to this fix for the test_app_started_client_configuration system test cherry-picked on top of 2374d120bbbc94109c449a0a9c182a9d4d0af60e

jandro996 pushed a commit that referenced this pull request Feb 29, 2024
* Collect null config defaults.
Properly serialize inner maps as string.

* Use snake_case for capture telemetry configuration

* Test common standardized settings
Add the log.injection.enabled setting with the log.injection alias
Add the trace.tags setting with the tags alias

* Empty maps and list config settings are collected as empty strings

* Render default Config Setting Value as strings

* Postpone config rendering as string until reporting phase

* default_system_tests_commit set to system-test fix #1546
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: telemetry Telemetry run-tests: all Run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants