refactor(otel): build providers in a postfork hook via worker_id dispatch (MAPCO-11222) - #88
Merged
Merged
Conversation
…hook Move the TracerProvider/MeterProvider construction out of import scope into _init_telemetry(), and dispatch on uwsgi.worker_id() at the bottom of the module to decide when to call it: - imported in the uWSGI master (lazy-app = false) -> defer via postfork - imported in a worker (lazy-app = true) -> initialise eagerly - not under uWSGI at all -> initialise eagerly Instrumentors are installed at import time and bind to ProxyTracer objects, which resolve once a real provider is set, so the explicit tracer_provider= kwargs are no longer needed and were removed. Both providers are flushed from an atexit handler so the final batch is exported when a worker recycles. This commit is behaviour-preserving under the chart's current lazy-app = true: the module is imported in the worker, so telemetry initialises eagerly exactly as before. Flipping lazy-app is a separate change. Also corrects the readme's claim that a master-initialised BatchSpanProcessor has its export thread die silently on fork. Verified false against SDK 1.44.0, where BatchSpanProcessor delegates to BatchProcessor in opentelemetry.sdk._shared_internal and an at-fork handler restarts the thread in the child. Refs: MAPCO-11222, MAPCO-11221
…claim Three review findings from PR #88. 1. uwsgidecorators raises a bare Exception, not ImportError, when the uWSGI master is disabled: if uwsgi.masterpid() == 0: raise Exception("you have to enable the uWSGI master process ...") The dispatch caught only ImportError, so any uWSGI run without `master = true` died while importing app.py — and with `need-app = true` uWSGI then refuses to boot. Before this refactor the module never imported uwsgi at all, so this was a regression. The deployed chart sets master = true, so dev and prod were unaffected; debug runs and consumer-supplied inis were not. It was also imported unconditionally, including on the worker branch where postfork is never used. Now imported only in the master branch, wrapped in `except Exception`, falling back to eager init so a masterless worker gets telemetry rather than none. 2. The atexit flush was documented as guaranteeing the final batch is exported on worker recycle. It does not. uWSGI's python plugin skips Py_Finalize(), and therefore all atexit handlers, when the worker is hijacked, is busy in a request, or runs async; SIGKILL paths (harakiri, worker-reload-mercy expiry) bypass it entirely. Softened to best-effort in both the readme and the _shutdown_telemetry docstring, with the conditions spelled out. 3. The readme described the postfork hook as the shipped path, which is not true while the chart has lazy-app = true. Reworded to lead with the worker_id dispatch and to stay accurate under either setting. Refs: MAPCO-11221, MAPCO-11222
…uilt one _init_telemetry() assigned the module global before calling set_tracer_provider / set_meter_provider, then reassigned it to a fresh fallback provider in the except handler. Both setters are FIRST-WINS -- a second call logs "Overriding of current TracerProvider is not allowed" and does nothing (verified against the SDK in the image). So if anything raised *after* the set succeeded -- a broken log handler from log.ini is the realistic trigger, since an _otel_log call sits between the set and the end of the block -- the global ended up pointing at an inert fallback while the real provider stayed globally installed. _shutdown_telemetry() then shut down the orphan and never flushed the live BatchSpanProcessor queue: the exact opposite of what the atexit handler exists to do. Both blocks now build into a local, and a finally clause binds the global from trace.get_tracer_provider() / metrics.get_meter_provider() so it always tracks whatever is actually installed. A Proxy*Provider means nothing was installed and has no shutdown(), so the global is left None -- which _shutdown_telemetry already skips. Reproduced against the image's SDK: under the old pattern the global was not the live provider; under the new one it is, and calling shutdown() on it flushes a span that the orphan would have dropped. Refs: MAPCO-11221, MAPCO-11222
CL-SHLOMIKONCHA
previously approved these changes
Aug 2, 2026
Collaborator
|
Please create ticket about the app.py telemetry support - separate relevant method from app.py) |
…CO-11223) (#89) * perf(uwsgi): set lazy-app = false so the master preforks the app Flip the chart's uwsgi ini to load mapproxy.yaml once in the master and fork workers from it, so the parsed config is shared copy-on-write instead of being parsed once per worker. Safe only on top of the _init_telemetry() dispatch: app.py detects that it was imported in the master and defers provider construction to a postfork hook, so each worker still gets its own export threads and its own gRPC channels. The replaced comment claimed the opposite of the value it annotated ("Fork workers after app load (required for OTel)" on lazy-app = true) and carried a stale todo. Refs: MAPCO-11223, MAPCO-11221 * Update helm/config/mapProxyUwsgi.ini Co-authored-by: Shlomi k <65117898+CL-SHLOMIKONCHA@users.noreply.github.com> --------- Co-authored-by: Shlomi k <65117898+CL-SHLOMIKONCHA@users.noreply.github.com>
Contributor
Author
CL-SHLOMIKONCHA
approved these changes
Aug 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
2 participants
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.
Related issues: MAPCO-11222, MAPCO-11221 (sub-tasks of MAPCO-11217)
Further information:
What this does
Moves
TracerProvider/MeterProviderconstruction out of import scope into_init_telemetry(), and adds a dispatch block at the bottom ofsrc/app.pythat decides when to call it based on which process did the import:uwsgi.worker_id()lazy-app = false)0postforkhooklazy-app = true)>= 1ImportErrorInstrumentors are still installed at import time. They bind to
ProxyTracerobjects that resolve once a real provider is set, so the seven explicittracer_provider=kwargs became redundant and were removed. Both providers are now flushed from anatexithandler so the final batch is exported on worker recycle.Why this is safe to merge on its own
This PR does not change runtime behaviour. The chart still ships
lazy-app = true, so the module is imported in the worker,worker_id()is>= 1, and telemetry initialises eagerly exactly as it does today. Thepostforkpath is dormant untillazy-appis flipped, which is the stacked follow-up PR.That split is deliberate: the dispatch is what removes the "must be atomic" constraint that MAPCO-11223 was originally written around.
Correction to previously documented behaviour
The readme claimed a master-initialised
BatchSpanProcessorhas its background export thread "die silently on fork". That is not accurate, and the claim is removed here.Verified against the SDK pinned in this image (1.44.0):
BatchSpanProcessordelegates toBatchProcessorinopentelemetry.sdk._shared_internal, which registers anos.register_at_forkhandler that restarts the export thread in the child. A span emitted in a forked child was exported with noforce_flush.PeriodicExportingMetricReaderis protected the same way, and_ProxyHistogramwas confirmed to resolve post-fork.Worth knowing if you re-check this: in SDK 1.7.1 the handler lived in
opentelemetry.sdk.trace.export, so grepping that module on a current SDK returns zero and makes the protection look absent. That module move is what produced the wrong claim in the first place.So the remaining justifications for building post-fork are narrower than the readme implied:
_shared_internalis private and the machinery has already moved once between versions.Testing
Built as
raster/docker-mapproxy:v2.0.0-postfork-testand deployed toraster-devas an isolated release. Withlazy-app = false(i.e. the follow-up PR's config) the load order was confirmed from logs:followed by the collector probe,
TracerProviderandMeterProviderinit lines once per worker (6x each). Tiles served byte-identical output to the deployedv1.9.2— same 2662-byte 256x256 PNG for the same WMTS tile.Not verified — stated explicitly so the green result is not over-read:
atexitflush on a real worker recycle.lazy-app = truearm as a controlled A/B, becauselazy-appis hardcoded in the chart ini rather than exposed as a Helm value. See MAPCO-11224.Review notes
app.pyin this repo, and CI hastestsandopenapi-lintgated off withif: false. This PR does not add a test harness; that would be a larger change than the refactor itself.