refactor: telemetry request parameters by parametalol · Pull Request #19722 · stackrox/stackrox

parametalol · 2026-03-31T20:55:25Z

Description

This is a prefactoring that simplifies implementation of matching requests by header name glob pattern.

In addition, gRPC metadata is now converted to http.Header type with canonicalized keys to support protocol-independent matching.

User-facing documentation

CHANGELOG.md is updated OR update is not needed
documentation PR is created and is linked above OR is not needed

Testing and quality

the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
CI results are inspected

Automated testing

How I validated my change

CI

Current dependencies on/for this PR:

master
- PR refactor: telemetry request parameters #19722 👈
  - PR ROX-33893: glob patterns for header name #19723

rhacs-bot · 2026-03-31T21:10:22Z

Images are ready for the commit at 117a534.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-528-g117a534f8b.

codecov · 2026-03-31T21:33:59Z

Codecov Report

❌ Patch coverage is 96.77419% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.61%. Comparing base (c3cccf6) to head (117a534).
⚠️ Report is 14 commits behind head on master.

Files with missing lines	Patch %	Lines
central/telemetry/centralclient/interceptors.go	66.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #19722      +/-   ##
==========================================
+ Coverage   49.59%   49.61%   +0.01%     
==========================================
  Files        2756     2760       +4     
  Lines      208036   208154     +118     
==========================================
+ Hits       103183   103271      +88     
- Misses      97192    97223      +31     
+ Partials     7661     7660       -1

Flag	Coverage Δ
go-unit-tests	`49.61% <96.77%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

parametalol · 2026-04-02T08:21:26Z

/test ocp-4-21-operator-e2e-tests

coderabbitai · 2026-04-02T09:04:33Z

📝 Walkthrough

Summary by CodeRabbit

Refactor
- Refactored telemetry campaign matching system with improved header pattern matching and request parameter handling for better internal consistency and maintainability.

Walkthrough

The pull request refactors telemetry header handling from function-based callbacks to concrete types. Changes include introducing a GlobMap type for header patterns, replacing HasHeader() with MatchHeaders() that returns matched headers instead of boolean results, updating campaign callback signatures to receive matched headers, and adding new header pattern-matching methods like GetMatching() and NewHeaders() constructor for gRPC metadata support.

Changes

Cohort / File(s)	Summary
Interceptor Header Updates `central/telemetry/centralclient/interceptors.go`, `pkg/telemetry/phonehome/interceptor.go`	Changed header handling from function-based callbacks to concrete `Headers` type; updated ServiceNow integration header constant casing; replaced `addCustomHeaders()` helper with direct header iteration using `phonehome.Headers`; modified `getGRPCRequestDetails()` and `getHTTPRequestDetails()` to return concrete `Headers` instead of per-key lookup functions.
Campaign Signature Changes `pkg/telemetry/phonehome/campaign.go`	Updated `APICallCampaignCriterion.Headers` from `map[string]glob.Pattern` to `GlobMap`; changed `MethodPattern()`, `PathPattern()`, and `HeaderPattern()` parameter types from `string` to `glob.Pattern`; modified `CountFulfilled()` callback signature to receive matched headers in addition to criterion; updated fulfillment logic to use `MatchHeaders()` returning `Headers` instead of boolean.
Headers Multimap Methods `pkg/telemetry/phonehome/headers_multimap.go`	Added `NewHeaders(metadata.MD)` constructor for gRPC metadata conversion; added `GetMatching(key string, value glob.Pattern)` method for pattern-based header retrieval with nil/absent-key semantics; removed standalone `GetFirst()` helper function.
Request Parameters Refactor `pkg/telemetry/phonehome/request_params.go`	Changed `RequestParams.Headers` from `func(string) []string` to concrete `Headers` type; introduced exported `GlobMap` type; replaced `HasHeader()` method with `MatchHeaders()` that returns matched `Headers` or `nil` on pattern mismatches.
Interceptor Tests `central/telemetry/centralclient/interceptors_test.go`, `pkg/telemetry/phonehome/interceptor_test.go`	Updated `withUserAgent()` helper to return concrete `Headers` instead of function; replaced test assertions using `Headers(...)` callable accessor with `.Get(key)` method calls; removed `Test_addCustomHeaders()` test entirely; added case-insensitive header key canonicalization assertion.
Campaign and Headers Tests `pkg/telemetry/phonehome/campaign_test.go`, `pkg/telemetry/phonehome/headers_multimap_test.go`, `pkg/telemetry/phonehome/request_params_test.go`	Updated test helpers and assertions to use concrete `Headers` type with `.Get()` method; replaced `TestHasHeader()` with `TestMatchHeaders()` verifying `Headers` return type and pattern-matching behavior; removed `TestGetFirst()`; added `TestGetMatching()` with table-driven cases for glob pattern matching; adjusted header pattern construction from `map[string]glob.Pattern` to `GlobMap`.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'refactor: telemetry request parameters' directly describes the main change - a refactoring of telemetry request parameter handling to support header-based pattern matching.
Description check	✅ Passed	The PR description covers the main objectives (prefactoring for header matching, gRPC metadata conversion), indicates testing approach (CI-tested with unit tests added/modified), and completes the required checklist items comprehensively.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

✅ Generated successfully - (🔄 Check to regenerate)
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch michael/telemetry-requestparam-refactoring

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (2)

central/telemetry/centralclient/interceptors.go (1)
14-14: Header constant casing changed.

The constant changed from Rh-ServiceNow-Integration to Rh-Servicenow-Integration. Since HTTP header lookup is case-insensitive and http.Header canonicalizes keys, this should work correctly for matching. However, if external systems expect the exact original casing in responses or logs, verify this change aligns with their expectations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@central/telemetry/centralclient/interceptors.go` at line 14, The header
constant snowIntegrationHeader in interceptors.go was altered from
"Rh-ServiceNow-Integration" to "Rh-Servicenow-Integration"; revert the constant
value back to "Rh-ServiceNow-Integration" (or confirm with integrators that the
new casing is acceptable) so external systems expecting the original exact
header casing in responses/logs are not broken; update the snowIntegrationHeader
definition accordingly and run tests to ensure no callers or logs rely on the
changed string.
pkg/telemetry/phonehome/campaign_test.go (1)
140-140: Typo in variable name.

expecedHeaders should be expectedHeaders.
Proposed fix
-		expecedHeaders := Headers{
+		expectedHeaders := Headers{
Also update the reference on line 155.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/telemetry/phonehome/campaign_test.go` at line 140, Rename the misspelled
test variable `expecedHeaders` to `expectedHeaders` (the variable is of type
`Headers`) and update any references to it in the same test (e.g., the assertion
or comparison that currently uses `expecedHeaders`) to use `expectedHeaders`
instead so the identifier matches the correct spelling.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@central/telemetry/centralclient/interceptors.go`:
- Line 14: The header constant snowIntegrationHeader in interceptors.go was
altered from "Rh-ServiceNow-Integration" to "Rh-Servicenow-Integration"; revert
the constant value back to "Rh-ServiceNow-Integration" (or confirm with
integrators that the new casing is acceptable) so external systems expecting the
original exact header casing in responses/logs are not broken; update the
snowIntegrationHeader definition accordingly and run tests to ensure no callers
or logs rely on the changed string.

In `@pkg/telemetry/phonehome/campaign_test.go`:
- Line 140: Rename the misspelled test variable `expecedHeaders` to
`expectedHeaders` (the variable is of type `Headers`) and update any references
to it in the same test (e.g., the assertion or comparison that currently uses
`expecedHeaders`) to use `expectedHeaders` instead so the identifier matches the
correct spelling.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: b73b4739-be94-4858-b7b8-7ca6e167f4ac

📥 Commits

Reviewing files that changed from the base of the PR and between c3cccf6 and 117a534.

📒 Files selected for processing (10)

central/telemetry/centralclient/interceptors.go
central/telemetry/centralclient/interceptors_test.go
pkg/telemetry/phonehome/campaign.go
pkg/telemetry/phonehome/campaign_test.go
pkg/telemetry/phonehome/headers_multimap.go
pkg/telemetry/phonehome/headers_multimap_test.go
pkg/telemetry/phonehome/interceptor.go
pkg/telemetry/phonehome/interceptor_test.go
pkg/telemetry/phonehome/request_params.go
pkg/telemetry/phonehome/request_params_test.go

coderabbitai · 2026-04-02T09:33:20Z

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #19777

@parametalol

Docstrings generation was requested by @parametalol. * #19722 (comment) The following files were modified: * `central/telemetry/centralclient/interceptors.go` * `pkg/telemetry/phonehome/campaign.go` * `pkg/telemetry/phonehome/headers_multimap.go` * `pkg/telemetry/phonehome/interceptor.go`

central/telemetry/centralclient/interceptors_test.go

pkg/telemetry/phonehome/campaign_test.go

@parametalol

Docstrings generation was requested by @parametalol. * #19722 (comment) The following files were modified: * `central/telemetry/centralclient/interceptors.go` * `pkg/telemetry/phonehome/campaign.go` * `pkg/telemetry/phonehome/headers_multimap.go` * `pkg/telemetry/phonehome/interceptor.go`

parametalol mentioned this pull request Mar 31, 2026

ROX-33893: glob patterns for header name #19723

Open

9 tasks

github-actions bot added the area/central label Mar 31, 2026

parametalol changed the title ~~michael/telemetry-requestparam-refactoring~~ refacotr: telemetry request parameters Mar 31, 2026

parametalol changed the title ~~refacotr: telemetry request parameters~~ refactor: telemetry request parameters Mar 31, 2026

parametalol mentioned this pull request Mar 31, 2026

ROX-33893: Support glob patterns in telemetry header names #19687

Closed

9 tasks

parametalol force-pushed the michael/telemetry-requestparam-refactoring branch from 2e708df to 27967f6 Compare April 1, 2026 07:58

parametalol marked this pull request as draft April 1, 2026 12:00

openshift-ci bot added the do-not-merge/work-in-progress label Apr 1, 2026

parametalol added 2 commits April 1, 2026 20:59

refactoring

9b89931

MatchHeaders refactoring

5f7202d

parametalol force-pushed the michael/telemetry-requestparam-refactoring branch from 2aa5f95 to 5f7202d Compare April 1, 2026 18:59

more tests

117a534

parametalol marked this pull request as ready for review April 1, 2026 21:44

openshift-ci bot removed the do-not-merge/work-in-progress label Apr 1, 2026

parametalol requested review from a team and stehessel April 2, 2026 08:14

parametalol added the coderabbit-review label Apr 2, 2026

coderabbitai bot reviewed Apr 2, 2026

View reviewed changes

coderabbitai bot mentioned this pull request Apr 2, 2026

docs(go): Add docstrings to phonehome telemetry methods #19777

Open

stehessel approved these changes Apr 2, 2026

View reviewed changes

central/telemetry/centralclient/interceptors_test.go Show resolved Hide resolved

pkg/telemetry/phonehome/campaign_test.go Show resolved Hide resolved

parametalol merged commit 994dff5 into master Apr 2, 2026
117 checks passed

parametalol deleted the michael/telemetry-requestparam-refactoring branch April 2, 2026 12:31

c-du pushed a commit that referenced this pull request Apr 2, 2026

refactor: telemetry request parameters (#19722)

3dd2e98

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: telemetry request parameters#19722

refactor: telemetry request parameters#19722
parametalol merged 3 commits intomasterfrom
michael/telemetry-requestparam-refactoring

parametalol commented Mar 31, 2026 •

edited

Loading

Uh oh!

rhacs-bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

codecov bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

parametalol commented Apr 2, 2026

Uh oh!

coderabbitai bot commented Apr 2, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot commented Apr 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

parametalol commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

User-facing documentation

Testing and quality

Automated testing

How I validated my change

Uh oh!

rhacs-bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

parametalol commented Apr 2, 2026

Uh oh!

coderabbitai bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

parametalol commented Mar 31, 2026 •

edited

Loading

rhacs-bot commented Mar 31, 2026 •

edited

Loading

codecov bot commented Mar 31, 2026 •

edited

Loading

coderabbitai bot commented Apr 2, 2026 •

edited

Loading

coderabbitai bot commented Apr 2, 2026 •

edited

Loading