Skip to content

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

Description

@brhenc

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggeneralGeneral issue, not specific to a particular contrib

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions