Skip to content

Refactor code to use upstream SDKStatsManager#182

Merged
rads-1996 merged 12 commits into
microsoft:mainfrom
rads-1996:use-upstream-sdkstats-manager
Jun 5, 2026
Merged

Refactor code to use upstream SDKStatsManager#182
rads-1996 merged 12 commits into
microsoft:mainfrom
rads-1996:use-upstream-sdkstats-manager

Conversation

@rads-1996
Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings June 1, 2026 23:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the distro’s SDKStats implementation to rely on the upstream Azure Monitor exporter StatsbeatManager for feature/instrumentation gauges, while adding distro-owned network success-count gauges that attach to the upstream statsbeat pipeline.

Changes:

  • Removed the distro’s standalone SdkStatsManager/SdkStatsMetrics implementation in favor of upstream StatsbeatManager.
  • Added a standalone-path StatsbeatConfig builder and a new _network_metrics module to export OTLP/A365 request success counts via upstream statsbeat.
  • Updated tests and distro initialization logic to reflect the new orchestration and idempotent gauge registration.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/test_sdkstats.py Updated/added tests for upstream singleton resets, config builder, network gauge callback/registration, and new _initialize_sdkstats behavior.
src/microsoft/opentelemetry/_sdkstats/_utils.py Updated comments to reflect that network gauges are now drained by _network_metrics.
src/microsoft/opentelemetry/_sdkstats/_network_metrics.py New module that attaches distro network success-count callback(s) to upstream statsbeat gauges.
src/microsoft/opentelemetry/_sdkstats/_metrics.py Removed the distro-owned observable gauge implementation.
src/microsoft/opentelemetry/_sdkstats/_manager.py Removed the distro-owned SDKStats manager.
src/microsoft/opentelemetry/_sdkstats/_config.py New helper to construct a default upstream StatsbeatConfig for standalone (non-Azure Monitor) scenarios.
src/microsoft/opentelemetry/_sdkstats/init.py Updated package documentation and removed exported manager symbol.
src/microsoft/opentelemetry/_distro.py Updated SDKStats initialization to bridge bits to upstream, initialize upstream manager in standalone mode, and register network gauges.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/microsoft/opentelemetry/_sdkstats/_network_metrics.py
Comment thread src/microsoft/opentelemetry/_sdkstats/_network_metrics.py Outdated
Comment thread src/microsoft/opentelemetry/_sdkstats/_config.py
Comment thread src/microsoft/opentelemetry/_distro.py Outdated
Comment thread tests/test_sdkstats.py
@rads-1996
Copy link
Copy Markdown
Member Author

A365 Only -

image

AzMon + A365 -

image

Comment thread src/microsoft/opentelemetry/_sdkstats/_config.py Outdated
Comment thread src/microsoft/opentelemetry/_distro.py Outdated
Comment thread src/microsoft/opentelemetry/_distro.py Outdated
Comment thread src/microsoft/opentelemetry/_distro.py Outdated
Comment thread src/microsoft/opentelemetry/_distro.py
Comment thread src/microsoft/opentelemetry/_sdkstats/_network_metrics.py Outdated
Comment thread src/microsoft/opentelemetry/_sdkstats/_network_metrics.py
Comment thread src/microsoft/opentelemetry/_sdkstats/_network_metrics.py Outdated
Comment thread src/microsoft/opentelemetry/_sdkstats/_network_metrics.py Outdated
Comment thread src/microsoft/opentelemetry/_sdkstats/_network_metrics.py
@rads-1996
Copy link
Copy Markdown
Member Author

A365 Only -

image

@rads-1996 rads-1996 force-pushed the use-upstream-sdkstats-manager branch from 44abc94 to 2aa7971 Compare June 3, 2026 17:18
JacksonWeber added a commit that referenced this pull request Jun 4, 2026
* ci(perf): post sticky comment for fork PRs via workflow_run

The Performance workflow runs in the PR context, which only grants a
read-only GITHUB_TOKEN for pull requests from forks. As a result the
"Post sticky PR comment" step was gated off for cross-repo PRs and the
perf comparison never appeared on those PRs (e.g. #182).

Move the comment posting into a new "Performance Comment" workflow
triggered by workflow_run, which executes in the base repository
context with pull-requests: write regardless of the PR's origin. The
benchmarking workflow now records the PR number in pr-number.txt and
uploads it alongside report.md so the follow-up workflow can target the
correct PR.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* ci(perf): harden comment workflow per review

Address Copilot review feedback on #186:

* Treat the perf-results artifact as untrusted. Check out the base repo
  and regenerate report.md here from base.json/pr.json using the
  base-branch copy of perf.compare so the markdown posted under the
  writable GITHUB_TOKEN is never attacker-supplied by a fork PR.
* Make actions/download-artifact non-fatal (continue-on-error) and gate
  every subsequent step on the download outcome, so a cancelled or
  failed upstream Performance run skips cleanly instead of marking the
  comment job as failed.
* Validate that pr-number.txt contains a positive integer via regex
  before passing it to the sticky-comment action; skip cleanly if not.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@JacksonWeber JacksonWeber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, replies to Leighton's comments make sense to me.

@rads-1996 rads-1996 force-pushed the use-upstream-sdkstats-manager branch from 3370b0a to 99d85cc Compare June 4, 2026 21:57
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 4, 2026

Performance comparison

Threshold: regressions >15.0% on gating scenarios fail the build. Higher ops/s is better; positive Δ means the PR is slower.

Scenario Gating Baseline (ops/s) Candidate (ops/s) Δ % Status
azure_monitor_log yes 26,433.3 25,612.8 +3.20%
azure_monitor_span yes 156,936.6 156,201.2 +0.47%
otel_log no 31,990.8 31,388.3 +1.92%
otel_span no 34,158.8 34,347.7 -0.55%

Comment thread src/microsoft/opentelemetry/_distro.py
@rads-1996 rads-1996 merged commit e12951f into microsoft:main Jun 5, 2026
11 checks passed
@rads-1996 rads-1996 deleted the use-upstream-sdkstats-manager branch June 5, 2026 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants