Onesettings local storage toggle

[lzchen]

Remote local-storage toggle: Local (offline) disk storage can be enabled/disabled at runtime via the OneSettings  FEATURE_LOCAL_STORAGE  flag, and never overrides an explicit  disable_offline_storage=True  user opt-out.

• Flag-based toggling: Disabling keeps the  LocalFileStorage  instance and its maintenance thread alive and just flips an  _active  flag so  put() / gets()  no-op — avoiding teardown/reconstruction churn and thread-restart races on re-enable.
• Late-exporter handling: Exporters created after a disabled state was already cached apply the cached kill-switch immediately on registration.
• Stats exporters: Statsbeat storage is decoupled from the user setting (always off); customer-sdkstats honors the user setting and follows the remote toggle (shared on-disk folder).
• Memory-leak fix: Bound-method OneSettings callbacks are stored via  weakref.WeakMethod , so discarded exporters are garbage-collected instead of pinned for the process lifetime; dead refs are pruned on notify.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
9 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Adds OneSettings-controlled runtime toggling for offline storage and aligns internal stats exporters with storage opt-out behavior.

Changes:

• Adds runtime storage enable/disable callbacks.
• Disables statsbeat persistence while propagating customer storage settings.
• Expands tests and release notes.

Reviewed changes

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

Show a summary per file

File	Description
tests/test_base_exporter.py	Tests storage toggling and callback registration.
tests/statsbeat/test_manager.py	Tests statsbeat storage configuration.
tests/customer_sdk_stats/test_manager.py	Tests customer SDK stats opt-out propagation.
tests/customer_sdk_stats/test_customer_sdkstats.py	Tests forwarding the storage setting.
CHANGELOG.md	Documents runtime storage toggling.
statsbeat/customer/_manager.py	Passes storage opt-out to the customer stats exporter.
statsbeat/customer/_customer_sdkstats.py	Forwards the originating exporter’s setting.
statsbeat/_manager.py	Disables statsbeat disk persistence.
export/_base.py	Implements storage callbacks and concurrent access handling.
_quickpulse/_live_metrics.py	Clarifies live-metrics re-enable gating.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py:239

• This registration cannot make the advertised remote toggle work: get_configuration_manager() only constructs the singleton and register_callback() only appends to its callback list. The only BaseExporter call to initialize() remains commented out above (lines 121–124 and 213–222), and there is no production initializer elsewhere in the repository, so no configuration worker ever polls OneSettings or invokes this callback. Start the manager with the exporter profile as part of the production lifecycle before relying on the callback.

        if not self._is_stats_exporter():
            config_manager = get_configuration_manager()
            if config_manager:
                config_manager.register_callback(self._local_storage_configuration_callback)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py:239

• Registering this bound method gives the process-wide manager a strong reference to every exporter, but the manager has no unregister operation and trace/log/metric shutdown() only closes storage. Consequently shut-down exporters cannot be collected and can still receive later toggles; after a prior disable, an enable callback can even create a new LocalFileStorage maintenance thread for an exporter that has already shut down. Add callback unregistration to exporter shutdown (or store weak callbacks) so the callback lifetime matches the exporter lifetime.

            config_manager = get_configuration_manager()
            if config_manager:
                config_manager.register_callback(self._local_storage_configuration_callback)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py:239

• Registration does not replay the manager's cached settings. Callbacks are notified only when a new settings payload is fetched, so an exporter created after FEATURE_LOCAL_STORAGE=disabled was cached starts with storage enabled and can remain that way indefinitely while change checks return 304. Make subscription and delivery of the current settings atomic so every new exporter immediately honors the active kill-switch without racing a concurrent update.

            config_manager = get_configuration_manager()
            if config_manager:
                config_manager.register_callback(self._local_storage_configuration_callback)

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py:239

• The singleton manager stores this bound method strongly, but exporter shutdown() only closes storage and never unregisters it; the callback list is cleared only when the entire configuration manager shuts down. Consequently every discarded exporter remains reachable for the manager's lifetime, and a later disable/enable notification can create a new storage thread for an exporter that was already shut down. Please add callback lifecycle cleanup (or register a weak callback) and prevent post-shutdown re-enablement.

        if not self._is_stats_exporter():
            config_manager = get_configuration_manager()
            if config_manager:
                config_manager.register_callback(self._local_storage_configuration_callback)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py:248

• Registering the callback and replaying get_settings() are not atomic with configuration updates. For example, this thread can read cached disabled, the worker can then publish and finish an enabled callback (which sees the initially active storage and does nothing), and this stale replay can finally disable storage, leaving the exporter opposite to the latest OneSettings state. Please make callback registration plus initial replay ordered with manager updates (for example, a manager API that serializes registration/replay with notifications).

                # once here so late-created exporters honor an existing kill-switch immediately. An
                # empty cache evaluates to a no-op, leaving storage per the user's setting.
                cached_settings = config_manager.get_settings()
                if cached_settings:
                    self._local_storage_configuration_callback(cached_settings)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/statsbeat/_manager.py:252

• This key decoupling behavior is not exercised by the updated tests: the new assertions only verify that StatsbeatConfig preserves the user's value, so they would still pass if this argument were changed back to config.disable_offline_storage. Add a _do_initialize/initialize test with config.disable_offline_storage=False that mocks AzureMonitorMetricExporter and asserts it is constructed with disable_offline_storage=True.

                # Statsbeat never persists its own envelopes to disk. It is best-effort internal
                # diagnostics, so it does not need disk-backed retry, and disabling storage avoids any
                # disk writes for users who opted out. config.disable_offline_storage reflects the
                # customer's config and is used only for DISK_RETRY reporting below.
                disable_offline_storage=True,

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py:226

• A cached FEATURE_LOCAL_STORAGE=disabled is applied only after _enable_local_storage() has constructed LocalFileStorage. Its constructor creates/checks the directory, runs maintenance, and starts a thread, so a late exporter still touches the disk before immediately closing storage. Read/evaluate the cached setting before creating storage so an already-active kill switch prevents initialization entirely.

        if not self._disable_offline_storage:
            self._enable_local_storage()

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py:736

• The shutdown guard is a check-then-act race. An enable callback can read _local_storage_shutdown == False, pause while shutdown sets it to True and observes storage is None, then resume and construct/assign a new LocalFileStorage; the exporter is shut down but its maintenance thread has been resurrected. Guard enable/disable/shutdown with the same lock and make the shutdown check plus storage assignment atomic.

        if self._local_storage_shutdown:
            return
        if self.storage is not None:
            return
        if self._storage_directory is None:
            self._storage_directory = _get_storage_directory(self._instrumentation_key or "")
        self.storage = LocalFileStorage(  # pyright: ignore

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (1)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py:248

• The register-then-replay sequence is not ordered with configuration updates. If get_settings() returns the old cache, the worker can publish and invoke this callback with a newer value before this constructor applies cached_settings; the constructor then overwrites the newer toggle and leaves storage stale until another configuration change. Please make callback registration plus initial replay atomic/versioned in _ConfigurationManager rather than composing register_callback() and get_settings() here.

                cached_settings = config_manager.get_settings()
                if cached_settings:
                    self._local_storage_configuration_callback(cached_settings)

[github-actions]

[Pilot] PR Pipeline Failure Analysis

A CI pipeline failed on this pull request. Here is an automated analysis of what went wrong and how to get the build green.

What failed

The Pylint check for azure-monitor-opentelemetry-exporter failed with exit code 24 (build link). Two violations were reported:

1. azure/monitor/opentelemetry/exporter/_storage.py:223 — R0911 too-many-return-statements: LocalFileStorage.put has 7 return statements (limit is 6).
2. azure/monitor/opentelemetry/exporter/statsbeat/customer/_manager.py:226 — C0301 line-too-long: Line is 140 characters long (limit is 120).

Both issues are in code added or modified by this PR.

Recommended next steps

• _storage.py:223: Refactor LocalFileStorage.put to reduce the number of return statements to ≤ 6. Consider extracting an inner helper or consolidating early-exit conditions.
• statsbeat/customer/_manager.py:226: Break the long line at line 226 (140 chars) into multiple lines so it stays within the 120-character limit.
• After making the fixes, run azpysdk pylint . from sdk/monitor/azure-monitor-opentelemetry-exporter/ locally to verify the check passes.
• See the CI troubleshooting guide: https://aka.ms/ci-fix
• Push new commits to address the failures; this comment updates automatically on the next failing run.

Raw pipeline analysis (azsdk ci analyze)

Failed Tasks
### Errors:
[azure-monitor-opentelemetry-exporter :: pylint]
  azure/monitor/opentelemetry/exporter/_storage.py:223:
    [R0911(too-many-return-statements), LocalFileStorage.put]
    Too many return statements (7/6)

  azure/monitor/opentelemetry/exporter/statsbeat/customer/_manager.py:226:
    [C0301(line-too-long), ]
    Line too long (140/120)

pylint check completed with exit code 24
[END 1/1] pylint :: azure-monitor-opentelemetry-exporter -> FAIL(24) in 21.69s

SUMMARY:
azure-monitor-opentelemetry-exporter  pylint  FAIL(24)  21.69s
Total checks: 1 | Failed: 1 | Worst exit code: 24

Pipeline: https://dev.azure.com/azure-sdk/public/_build/results?buildId=6645846

Copilot detected the failing pipeline and generated the analysis above. To have it attempt a fix automatically, reply with @copilot please fix the failing pipeline on this PR.

Generated by Pipeline Analysis - Next Steps · 27 AIC · ⌖ 8.64 AIC · ⊞ 6.6K · ◷

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (4)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py:248

• The cached-state replay is not ordered with worker notifications. If this thread reads a disabled snapshot, the worker can publish and invoke the callback with a newer enabled state, and then this thread applies the stale disabled snapshot, leaving storage in the wrong state until another configuration change. Registration and replay need to be atomic/ordered inside the configuration manager (for example with a generation-aware register-and-replay API).

                cached_settings = config_manager.get_settings()
                if cached_settings:
                    self._local_storage_configuration_callback(cached_settings)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/statsbeat/customer/_customer_sdkstats.py:25

• This setting is only propagated for the first exporter: collect_customer_sdkstats skips initialize once the singleton manager is active. If exporter A enables storage and exporter B later specifies disable_offline_storage=True, B's SDK stats still feed the existing disk-backed exporter, so its explicit opt-out is not honored. The singleton must adopt the strongest opt-out or reconfigure when a later exporter disables storage.

        customer_stats.initialize(
            connection_string=exporter._connection_string,  # type: ignore
            credential=exporter._credential,  # type: ignore
            disable_offline_storage=exporter._disable_offline_storage,  # type: ignore

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_base_exporter.py:336

• This cleanup does not stop LocalFileStorage's PeriodicTask, so every new test that constructs a BaseExporter leaves a daemon maintenance thread running for the rest of the test process (the same pattern is repeated in the new cleanup blocks below). Close the storage before cleaning its path.

        finally:
            if base.storage is not None:
                clean_folder(base.storage._path)

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_configuration/init.py:87

• Dead WeakMethod entries are pruned only when _notify_callbacks runs. With a stable configuration—or currently, while manager initialization remains disabled—exporter churn continuously appends dead weak references and the singleton list still grows without bound; the eventual per-entry list.remove cleanup is also quadratic. Prune opportunistically during registration or use weakref finalizers/a single-pass rebuild so the leak fix does not leave an unbounded callback registry.

        if inspect.ismethod(callback):
            self._callbacks.append(weakref.WeakMethod(callback))
        else:
            self._callbacks.append(callback)

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.