fix: correctly merge statsbeat feature flags from JSON env var#162
Open
hectorhdzg wants to merge 1 commit into
Open
fix: correctly merge statsbeat feature flags from JSON env var#162hectorhdzg wants to merge 1 commit into
hectorhdzg wants to merge 1 commit into
Conversation
The statsbeat feature bitmap stored in AZURE_MONITOR_STATSBEAT_FEATURES is written as JSON (via JSON.stringify), but the merge logic read it back with Number(), which always returns NaN for JSON strings. This silently discarded all previously-set feature flags on every export cycle, causing statsbeat telemetry to lose feature-usage data for all customers. The fix parses both formats: - Plain number strings (backward compat with the shim) - JSON objects (written by setSdkStatsFeatures and patchOpenTelemetryInstrumentationEnable) Also fixes test isolation in main.test.ts where the env var leaked between tests, and adds regression tests for the JSON-format merge path.
hectorhdzg
requested review from
JacksonWeber,
MSNev,
jeremydvoss,
lzchen and
rads-1996
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes loss of Statsbeat feature-usage flags by correctly merging AZURE_MONITOR_STATSBEAT_FEATURES when it is stored as JSON (and preserving backward-compat for plain numeric strings). This ensures feature bits set by previous cycles/callers are not silently discarded.
Changes:
- Update env-var merge logic to support both numeric-string and JSON-formatted
AZURE_MONITOR_STATSBEAT_FEATURES. - Add unit regression tests covering the JSON env-var merge path and consecutive calls.
- Improve unit test isolation by clearing
AZURE_MONITOR_STATSBEAT_FEATURESbetween tests inmain.test.ts.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/utils/sdkStats.ts |
Fixes feature-bit merge by parsing JSON env var (fallback from numeric parsing). |
test/internal/unit/sdkStats.test.ts |
Adds regression tests for JSON merge and preservation across calls. |
test/internal/unit/main.test.ts |
Clears statsbeat env var in beforeEach to avoid cross-test leakage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
63
to
66
| beforeEach(() => { | ||
| originalEnv = process.env; | ||
| delete process.env[AZURE_MONITOR_STATSBEAT_FEATURES]; | ||
| // Preserve whatever the global OTel API object looks like before each test |
Comment on lines
+100
to
+114
| it("should not lose feature flags across consecutive setSdkStatsFeatures calls", () => { | ||
| const sb = getInstance(); | ||
|
|
||
| // First call sets OTLP | ||
| sb.setSdkStatsFeatures({}, { otlp: true }); | ||
| const firstOutput = JSON.parse(String(process.env[AZURE_MONITOR_STATSBEAT_FEATURES])); | ||
| expect(Number(firstOutput.feature) & SdkStatsFeature.OTLP).toBeTruthy(); | ||
|
|
||
| // Second call sets A365 — OTLP should be preserved via env var merge | ||
| sb.setSdkStatsFeatures({}, { a365: true }); | ||
| const secondOutput = JSON.parse(String(process.env[AZURE_MONITOR_STATSBEAT_FEATURES])); | ||
| const features = Number(secondOutput.feature); | ||
| expect(features & SdkStatsFeature.OTLP).toBeTruthy(); | ||
| expect(features & SdkStatsFeature.A365).toBeTruthy(); | ||
| }); |
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.
The statsbeat feature bitmap stored in AZURE_MONITOR_STATSBEAT_FEATURES is written as JSON (via JSON.stringify), but the merge logic read it back with Number(), which always returns NaN for JSON strings. This silently discarded all previously-set feature flags on every export cycle, causing statsbeat telemetry to lose feature-usage data for all customers.
The fix parses both formats:
Also fixes test isolation in main.test.ts where the env var leaked between tests, and adds regression tests for the JSON-format merge path.