Add env/stage filter for DDog and collapse more API URLs into single templates#4924
Merged
lukaszgryglicki merged 4 commits intodevfrom Mar 5, 2026
Merged
Conversation
…templates Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Datadog “env/stage” filtering support to the local span-querying utilities and continues unifying API route templating so span routes group consistently.
Changes:
check_spans_in_ddog.sh: adds strict bash settings, argument parsing for env/service, and builds the Datadog search payload viajq.api_usage_stats_ddog.py: introduces--env/--stageand appendsenv:<value>to the query unless already present.- Backend path sanitizers: updates route-templating logic in both Python and Go (but introduces a duplicate replacement call in each).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| utils/otel_dd/check_spans_in_ddog.sh | Adds env/service CLI args and safer JSON payload construction for Datadog span search. |
| utils/otel_dd/api_usage_stats_ddog.py | Adds env/stage option and auto-appends env: to the Datadog query when missing. |
| cla-backend/cla/routes.py | Adjusts API path sanitization (currently includes a duplicated numeric-id substitution). |
| cla-backend-go/telemetry/datadog_otlp.go | Adjusts Go-side path sanitization to match templating (currently includes a duplicated numeric-id replacement). |
WalkthroughAdds duplicate numeric ID-masking calls in two backend files (redundant replacements) and enhances Datadog utilities with environment-filter detection, new CLI/environment options, and safer jq-based payload construction for Datadog queries. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cla-backend/cla/routes.py (1)
111-111: Drop the duplicate numeric-ID substitution.Line 111 duplicates Line 110 with the same pattern/replacement, so this second pass has no effect.
Proposed cleanup
p = _RE_UUID_VALID.sub("{uuid}", p) p = _RE_UUID_LIKE.sub(r"/{invalid-uuid}\1", p) p = _RE_UUID_HEXDASH_36.sub(r"/{invalid-uuid}\1", p) p = _RE_NUMERIC_ID.sub(r"/{id}\1", p) - p = _RE_NUMERIC_ID.sub(r"/{id}\1", p) p = _RE_SFID_VALID.sub(r"/{sfid}\1", p)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cla-backend/cla/routes.py` at line 111, Remove the redundant second substitution call that applies the numeric-ID regex to p; the line invoking _RE_NUMERIC_ID.sub(r"/{id}\1", p) is a duplicate of the previous line and should be deleted so only the first substitution on variable p remains (search for the two consecutive calls to _RE_NUMERIC_ID.sub in the function/class that builds p and remove the latter).cla-backend-go/telemetry/datadog_otlp.go (1)
194-194: Remove the redundant numeric-ID replacement pass.Line 194 repeats the exact same substitution from Line 193; it is a no-op and adds maintenance noise.
Proposed cleanup
p = reUUIDLike.ReplaceAllString(p, "/{invalid-uuid}$1") p = reUUIDHexDash36.ReplaceAllString(p, "/{invalid-uuid}$1") p = reNumericID.ReplaceAllString(p, "/{id}$1") - p = reNumericID.ReplaceAllString(p, "/{id}$1") // Salesforce IDs: valid vs invalid🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cla-backend-go/telemetry/datadog_otlp.go` at line 194, The code calls reNumericID.ReplaceAllString twice in a row on variable p (the second call repeating the exact substitution), so remove the redundant second invocation — keep only a single p = reNumericID.ReplaceAllString(p, "/{id}$1") call (where p and reNumericID are used) to eliminate the no-op duplicate and reduce maintenance noise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@utils/otel_dd/api_usage_stats_ddog.py`:
- Around line 36-39: The env-filter regex (_ENV_RE) and its usage in
has_env_filter incorrectly miss common forms like "(env:prod)" and "-env:prod";
update _ENV_RE so it matches "env:" when preceded by start-of-string,
whitespace, a parenthesis, or a negative sign (e.g., account for "(" and "-"
before "env"), then keep has_env_filter(query: str) returning
bool(_ENV_RE.search(query)) so Line 209–210 won't append a duplicate env:
filter.
---
Nitpick comments:
In `@cla-backend-go/telemetry/datadog_otlp.go`:
- Line 194: The code calls reNumericID.ReplaceAllString twice in a row on
variable p (the second call repeating the exact substitution), so remove the
redundant second invocation — keep only a single p =
reNumericID.ReplaceAllString(p, "/{id}$1") call (where p and reNumericID are
used) to eliminate the no-op duplicate and reduce maintenance noise.
In `@cla-backend/cla/routes.py`:
- Line 111: Remove the redundant second substitution call that applies the
numeric-ID regex to p; the line invoking _RE_NUMERIC_ID.sub(r"/{id}\1", p) is a
duplicate of the previous line and should be deleted so only the first
substitution on variable p remains (search for the two consecutive calls to
_RE_NUMERIC_ID.sub in the function/class that builds p and remove the latter).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 831c09ce-3d8a-4023-b1ad-2fa7f8e6df39
📒 Files selected for processing (4)
cla-backend-go/telemetry/datadog_otlp.gocla-backend/cla/routes.pyutils/otel_dd/api_usage_stats_ddog.pyutils/otel_dd/check_spans_in_ddog.sh
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
There was a problem hiding this comment.
♻️ Duplicate comments (1)
utils/otel_dd/api_usage_stats_ddog.py (1)
39-42:⚠️ Potential issue | 🟠 MajorEnv-filter detection misses common Datadog query forms.
The regex
r"(^|\s)env\s*:"only matchesenv:at string start or after whitespace. Common query patterns like(env:prod)or-env:prodwon't be detected, causing the code at Lines 212-213 to append a duplicateenv:filter unexpectedly.🐛 Proposed fix to broaden the pattern
-_ENV_RE = re.compile(r"(^|\s)env\s*:") +_ENV_RE = re.compile(r"(?:^|[\s(\-])env\s*:", re.IGNORECASE)This matches
env:when preceded by start-of-string, whitespace, opening parenthesis, or hyphen (negation).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/otel_dd/api_usage_stats_ddog.py` around lines 39 - 42, The current env-detection regex `_ENV_RE = re.compile(r"(^|\s)env\s*:")` in function `has_env_filter` misses forms like `(env:prod)` or `-env:prod`; update the pattern to treat start-of-string OR any of whitespace, opening parenthesis, or hyphen as valid preceders (e.g. use a character class like `(^|[\s\(\-])env\s*:`) so `has_env_filter(query: str)` correctly detects `env:` when preceded by `(` or `-` as well as whitespace/start.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@utils/otel_dd/api_usage_stats_ddog.py`:
- Around line 39-42: The current env-detection regex `_ENV_RE =
re.compile(r"(^|\s)env\s*:")` in function `has_env_filter` misses forms like
`(env:prod)` or `-env:prod`; update the pattern to treat start-of-string OR any
of whitespace, opening parenthesis, or hyphen as valid preceders (e.g. use a
character class like `(^|[\s\(\-])env\s*:`) so `has_env_filter(query: str)`
correctly detects `env:` when preceded by `(` or `-` as well as
whitespace/start.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0308d40c-fc0f-43df-b10d-48cd4c6b0356
📒 Files selected for processing (2)
cla-backend/cla/tests/unit/test_jwt_auth.pyutils/otel_dd/api_usage_stats_ddog.py
✅ Files skipped from review due to trivial changes (1)
- cla-backend/cla/tests/unit/test_jwt_auth.py
ahmedomosanya
approved these changes
Mar 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cc @mlehotskylf
Signed-off-by: Lukasz Gryglicki lgryglicki@cncf.io
Assisted by OpenAI
Assisted by GitHub Copilot