test(debugger): add snapshot-correlation gate and Go weblog endpoints - #7425
test(debugger): add snapshot-correlation gate and Go weblog endpoints#7425grantseltzer wants to merge 1 commit into
Conversation
|
|
There was a problem hiding this comment.
More details
The new correlation chain returns the expected value, and the loop handler safely returns Loop 0 for both zero and negative counts. The shared line-probe mapping targets the newly added loop body and sibling lines consistently across Go weblog variants.
📊 Validated against 3 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 8431afa · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
nccatoni
left a comment
There was a problem hiding this comment.
LGTM (from @DataDog/system-tests-core) but you should get a review from someone familiar with the feature
| @scenarios.debugger_probes_snapshot | ||
| @slow | ||
| class Test_Debugger_Coordinated_Sampling(debugger.BaseDebuggerTest): | ||
| """Tier 1: the sampling decision is made once per trace and applies to every probe that |
There was a problem hiding this comment.
I don't think this docstring is providing a lot of value on the test suite
| for probe in probes: | ||
| probe["id"] = debugger.generate_probe_id("log") | ||
| # Use the tracer's existing per-probe sampling config to sample below 100%. | ||
| probe["sampling"] = {"snapshotsPerSecond": _SNAPSHOTS_PER_SECOND} |
There was a problem hiding this comment.
Why do we need to configure this?
There was a problem hiding this comment.
If you meant to point at dynamic_sampling_enabled=True that shouldn't have been added and I removed that change.
For this one, the test asserts at least one trace was fully dropped.
|
|
||
| from utils import scenarios, features, slow | ||
|
|
||
| # Snapshot rate for the correlation probes, set through the tracer's existing per-probe sampling |
There was a problem hiding this comment.
Do these constants need comments explaining them? I think the reader can infer what _SNAPSHOTS_PER_SECOND is...
|
|
||
|
|
||
| def _trace_id(snapshot: dict) -> str | None: | ||
| return snapshot.get("dd", {}).get("trace_id") |
There was a problem hiding this comment.
It does not, removed
| @features.debugger_probe_budgets | ||
| @scenarios.debugger_probes_snapshot | ||
| @slow | ||
| class Test_Debugger_Per_Span_Budget(debugger.BaseDebuggerTest): |
There was a problem hiding this comment.
Does it really make sense to break these into separate classes?
| @@ -0,0 +1,267 @@ | |||
| # Unless explicitly stated otherwise all files in this repository are licensed under the the Apache License Version 2.0. | |||
| # This product includes software developed at Datadog (https://www.datadoghq.com/). | |||
| # Copyright 2021 Datadog, Inc. | |||
There was a problem hiding this comment.
It's 2021 everywhere in this repo, are you sure it should be 26?
8431afa to
86f3832
Compare
|
Bits found no code fix to apply🟢 Investigated · ⚪ No code fix needed
View in Datadog | Reviewed commit 86f3832 · Any feedback? Reach out in #deveng-pr-agent |
Motivation
Changes
Workflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present