refactor(sdkstats): consolidate shared constants under sdkstats/const…#165
Merged
JacksonWeber merged 3 commits intoJun 2, 2026
Merged
Conversation
…ants.ts Addresses Radhika's follow-up suggestions on PR microsoft#156: - Move OTLP wrapper magic constants (endpoint category, unknown-status sentinel, retryable-network error codes, exception-type labels, OTLP retryable HTTP statuses) into a new src/sdkstats/constants.ts. - Move the wire-format metric-name constants (REQUEST_SUCCESS_NAME / REQUEST_FAILURE_NAME / REQUEST_DURATION_NAME / RETRY_COUNT_NAME / THROTTLE_COUNT_NAME / EXCEPTION_COUNT_NAME), the NETWORK_METRIC_NAMES tuple, and the HTTP status-code buckets out of networkStats.ts into the same constants module. networkStats.ts re-exports them for backwards compatibility. - Have the A365 exporter use the shared A365_ENDPOINT_CATEGORY label and the EXC_TIMEOUT/EXC_NETWORK/EXC_CLIENT exceptionType constants instead of duplicating the literal strings. - Document why we cannot just import StatsbeatCounter from @azure/monitor-opentelemetry-exporter (the enum lives at dist/{esm,commonjs}/export/statsbeat/types and the package's exports field only publishes '.' and './package.json', so the enum is not part of the public surface; we mirror its values and keep them in lockstep). All 923 tests pass, lint clean, build green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JacksonWeber
requested review from
MSNev,
ajmfehr,
biswapm,
fpfp100,
gwharris7,
hectorhdzg,
jeremydvoss,
juliomenendez,
lzchen,
nikhilNava,
rads-1996 and
sellakumaran
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR centralizes SDKStats network/exporter constant values into a new src/sdkstats/constants.ts module and refactors existing SDKStats components/exporters to consume those shared definitions, reducing duplication and drift risk across implementations.
Changes:
- Added
src/sdkstats/constants.tsas the single source of truth for wire-format metric names, status-code buckets, endpoint categories, and bounded exception labels. - Refactored
src/sdkstats/networkStats.tsandsrc/sdkstats/otlpWrapper.tsto import (and innetworkStats.ts, re-export) constants from the new module. - Updated
src/a365/exporter/Agent365Exporter.tsto use centralized endpoint category + exception label constants.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/sdkstats/constants.ts | Introduces centralized constants/types for SDKStats network metrics + classification. |
| src/sdkstats/networkStats.ts | Imports from constants.ts and re-exports key public symbols for backward compatibility. |
| src/sdkstats/otlpWrapper.ts | Removes local constant duplication and imports shared constants from constants.ts. |
| src/a365/exporter/Agent365Exporter.ts | Replaces inline endpoint/exception label strings with centralized constants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rads-1996
reviewed
Jun 2, 2026
…sbeatCounter cannot be imported
- {@link} paths were written as if from a parent of src/sdkstats/; corrected to be relative to src/sdkstats/constants.ts itself (./networkStats, ./otlpWrapper, ../a365/exporter/Agent365Exporter).
- Expanded the StatsbeatCounter rationale with the exact TS2307 error produced under our moduleResolution=NodeNext config and pointed at the upstream Azure SDK repo for the public-export ask.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rads-1996
approved these changes
Jun 2, 2026
…icating string literals Updates the four SDKStats test files (networkStats, otlpWrapper, metrics, agent365NetworkStats) to import and use the constants now exposed from sdkstats/constants.ts (A365_ENDPOINT_CATEGORY, OTLP_ENDPOINT_CATEGORY, OTLP_UNKNOWN_STATUS, OTLP_HTTP_RETRYABLE_STATUSES, EXC_TIMEOUT/NETWORK/CLIENT) instead of duplicating the literal strings inline. The single test that intentionally pins the wire-format metric-name strings to their literal values (networkStats.test.ts \�xposes all six SDKStats network metric names\) is preserved so a constant rename still fails loudly. Also rewrites the OTLP_HTTP_RETRYABLE_STATUSES iteration to drive the loop directly from the shared set so adding/removing a status updates both the wrapper and the test in lockstep. 923 tests pass, lint clean, build green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rads-1996
approved these changes
Jun 2, 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.
This pull request introduces a new
constants.tsmodule to centralize all shared constants for SDKStats network metrics, status code buckets, endpoint categories, and bounded exception labels. The main goal is to ensure a single source of truth for these values, which were previously duplicated across multiple files. The changes also refactor existing modules to import these constants from the new location, improving maintainability and consistency.Centralization of SDKStats constants:
src/sdkstats/constants.tsfile that defines and documents all wire-format metric names, HTTP status code buckets, endpoint category labels, and bounded exception type strings. This file is now the authoritative source for these values across the codebase.Refactoring to use centralized constants:
src/sdkstats/networkStats.tsto remove local constant definitions and instead re-export and import all relevant constants and types fromconstants.ts. This ensures backwards compatibility and reduces duplication. [1] [2]src/sdkstats/otlpWrapper.tsto remove its local constant definitions and import all endpoint, status, and exception-related constants fromconstants.ts.src/a365/exporter/Agent365Exporter.tsto import and use the centralized endpoint category and exception label constants fromconstants.ts, ensuring consistent labeling and error classification. [1] [2] [3]These changes will make it easier to keep SDKStats metric names and classification logic in sync across different exporter implementations and reduce the risk of subtle bugs caused by inconsistent constant values.