open-telemetry / open-telemetry/opentelemetry-cpp-contrib

[BUG] Worker process freezes for ~10s on `nginx -s reload` when the OTLP collector is unresponsive (missing exit_process hook, unconfigurable exporter timeout)

Open
#685 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug general
Dominant language
Python
Stars
153
Forks
184
Avg merge
2d 9h
Merged PRs (30d)
26

Description

Describe your environment

instrumentation/nginx (otel_ngx_module), current main. Read at source level against this repo, cross-checked against a confirmed opentelemetry-cpp SDK bug (open-telemetry/opentelemetry-cpp#4532).

Steps to reproduce

Two things compound to produce this:

  1. otel_ngx_module's lifecycle table never registers a worker-exit hook (instrumentation/nginx/src/otel_ngx_module.cpp):
ngx_module_t otel_ngx_module = {
  NGX_MODULE_V1,
  &otel_ngx_http_module,
  kOtelNgxCommands,
  NGX_HTTP_MODULE,
  nullptr,      /* init master */
  nullptr,      /* init module - prior to forking from master process */
  OtelNgxStart, /* init process - worker process fork */
  nullptr,      /* init thread */
  nullptr,      /* exit thread */
  nullptr,      /* exit process - worker process exit */   <-- no cleanup hook at all
  nullptr,      /* exit master */
  NGX_MODULE_V1_PADDING
};

OtelNgxStart sets up the TracerProvider/BatchSpanProcessor on worker fork, but nothing runs a bounded, explicit Shutdown() when the worker exits. Teardown is left entirely to whatever happens when the process's static storage is destroyed during exit(0).

  1. CreateExporter() builds the OTLP HTTP exporter with default options and never exposes a way to change the timeout (instrumentation/nginx/src/otel_ngx_module.cpp):
static std::unique_ptr<sdktrace::SpanExporter> CreateExporter(const OtelNgxAgentConfig* conf) {
  std::unique_ptr<sdktrace::SpanExporter> exporter;

  std::string endpoint = conf->exporter.endpoint;
  otlp::OtlpHttpExporterOptions opts;
  opts.url = endpoint.empty() ? opts.url : endpoint;   // <-- only url is ever set
  exporter.reset(new otlp::OtlpHttpExporter(opts));

  return exporter;
}

opts.timeout is never touched, so it keeps OtlpHttpExporterOptions's default (10 seconds), and there is no opentelemetry_exporter_timeout-style directive anywhere in agent_config.h / the config parsing to change it.

Recipe to observe the effect:

load_module otel_ngx_module.so;
events { worker_connections 1024; }
http {
    opentelemetry_service_name "nginx-test";
    opentelemetry_otlp_traces_endpoint "http://127.0.0.1:4318/v1/traces";
    opentelemetry_span_processor "batch";
    server {
        listen 8088;
        location / {
            opentelemetry on;
            return 200 "ok\n";
        }
    }
}
  1. Point opentelemetry_otlp_traces_endpoint at a listener that accepts the TCP connection and never responds (a "blackhole" collector -- e.g. a bare socket.listen() that just sleeps after accept()).
  2. Send one request to nginx so a span gets queued in the BatchSpanProcessor.
  3. Run nginx -s reload.

What is the expected behavior?

The old worker should exit promptly once it has drained its active connections -- a reload shouldn't be gated on a background telemetry export that has nothing to do with serving traffic.

What is the actual behavior?

The old worker sits in nginx: worker process is shutting down for the duration of the exporter's own request timeout (10s by default here, per point 2 above) before it can exit. This isn't a guess: open-telemetry/opentelemetry-cpp#4532 measures the exact mechanism directly -- BatchSpanProcessor::Shutdown() performs an unconditional, untimed worker_thread_.join() before it even looks at the timeout argument, so if the worker thread is mid-Export() against an unresponsive collector when shutdown is requested, the join blocks for as long as that Export() call takes to fail (measured: 9702ms wall-clock for a Shutdown(std::chrono::microseconds(1)) call against a real blackhole listener). Without an exit_process hook giving nginx's shutdown path any say in when/how that happens, and with no way to configure a shorter exporter timeout, every reload while the collector is down or slow pays this cost per worker.

In environments that reload frequently (Kubernetes Ingress Controllers, canary deployments, config-driven autoscaling), old workers can accumulate in this state faster than they clear, holding open listening sockets and file descriptors.

Additional context

Suggested shape (two independent, additive changes):

  1. Register an exit_process hook that calls the TracerProvider's Shutdown() with an explicit, short deadline before the worker exits, so at least the module makes a bounded attempt rather than leaving teardown entirely to static-storage destruction order.
  2. Expose an opentelemetry_exporter_timeout (and/or opentelemetry_shutdown_timeout) directive so operators who know their collector can be slow/unreachable aren't stuck with a hardcoded 10s budget with no way to tighten it.

Neither of these alone fully closes the gap -- opentelemetry-cpp#4532 is the reason even a Shutdown(1 microsecond) call from an exit_process hook doesn't actually return in anything close to 1 microsecond today. Filing both as companion reports since the fix likely needs to land on both sides: a shorter/configurable timeout here reduces the exposure window, and the SDK-level fix (once one exists) is what actually makes a short Shutdown() call keep its promise.

Happy to send a patch for the two additive changes above (the hook + the config directive) if that shape looks useful independent of how #4532 gets resolved.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with instrumentation/nginx/src/otel_ngx_module.cpp, especially the module lifecycle table, OtelNgxStart, and CreateExporter(), then inspect agent_config.h and the configuration parsing. Reproduce the blackhole-collector reload scenario and verify that worker shutdown is explicitly bounded and the exporter or shutdown timeout can be configured without the default 10-second delay.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, nginx
Domain
backend, observability
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.