Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support concurrent exports #781

Merged
merged 11 commits into from
May 17, 2022
Merged

Commits on May 5, 2022

  1. Add support for concurrent exports

    Applications generating significant span volume can end up dropping data
    due to the synchronous export step. According to the opentelemetry spec,
    
        This function will never be called concurrently for the same exporter
        instance. It can be called again only after the current call returns.
    
    However, it does not place a restriction on concurrent I/O or anything
    of that nature. There is an [ongoing discussion] about tweaking the
    language to make this more clear.
    
    With that in mind, this commit makes the exporters return a future that
    can be spawned concurrently. Unfortunately, this means that the
    `export()` method can no longer be async while taking &mut self. The
    latter is desirable to enforce the no concurrent calls line of the spec,
    so the choice is made here to return a future instead with the lifetime
    decoupled from self. This resulted in a bit of additional verbosity, but
    for the most part the async code can still be shoved into an async fn
    for the ergonomics.
    
    The main exception to this is the `jaeger` exporter which internally
    requires a bunch of mutable references. I plan to discuss with the
    opentelemetry team the overall goal of this PR and get buy-in before
    making more invasive changes to support this in the jaeger exporter.
    
    [ongoing discussion]: open-telemetry/opentelemetry-specification#2434
    jwilm committed May 5, 2022
    Configuration menu
    Copy the full SHA
    d5d7692 View commit details
    Browse the repository at this point in the history
  2. SpanProcessor directly manages concurrent exports

    Prior, export tasks were run in "fire and forget" mode with
    runtime::spawn. SpanProcessor now manages tasks directly using
    FuturesUnordered. This enables limiting overall concurrency (and thus
    memory footprint). Additionally, flush and shutdown logic now spawn an
    additional task for any unexported spans and wait on _all_ outstanding
    tasks to complete before returning.
    jwilm committed May 5, 2022
    Configuration menu
    Copy the full SHA
    293563d View commit details
    Browse the repository at this point in the history
  3. Add configuration for BSP max_concurrent_exports

    Users may desire to control the level of export concurrency in the batch
    span processor. There are two special values:
    
        max_concurrent_exports = 0: no bound on concurrency
        max_concurrent_exports = 1: no concurrency, makes everything
        synchronous on the messaging task.
    jwilm committed May 5, 2022
    Configuration menu
    Copy the full SHA
    56db3bb View commit details
    Browse the repository at this point in the history
  4. Implement new SpanExporter API for Jaeger

    Key points
    - decouple exporter from uploaders via channel and spawned task
    - some uploaders are a shared I/O resource and cannot be multiplexed
        - necessitates a task queue
        - eg, HttpClient will spawn many I/O tasks internally, AgentUploader
          is a single I/O resource. Different level of abstraction.
    - Synchronous API not supported without a Runtime argument. I updated
      the API to thread one through, but maybe this is undesirable. I'm also
      exploiting the fact in the Actix examples that it uses Tokio under the
      hood to pass through the Tokio runtime token.
    - Tests pass save for a couple of flakey environment ones which is
      likely a race condition.
    jwilm committed May 5, 2022
    Configuration menu
    Copy the full SHA
    a8d1e46 View commit details
    Browse the repository at this point in the history

Commits on May 11, 2022

  1. Reduce dependencies on futures

    The minimal necessary futures library (core, util, futures proper) is
    now used in all packages touched by the concurrent exporters work.
    jwilm committed May 11, 2022
    Configuration menu
    Copy the full SHA
    1571e21 View commit details
    Browse the repository at this point in the history
  2. Remove runtime from Jaeger's install_simple

    To keep the API _actually_ simple, we now leverage a thread to run the
    jaeger exporter internals.
    jwilm committed May 11, 2022
    Configuration menu
    Copy the full SHA
    33555df View commit details
    Browse the repository at this point in the history
  3. Add Arc lost in a rebase

    jwilm committed May 11, 2022
    Configuration menu
    Copy the full SHA
    cd8e054 View commit details
    Browse the repository at this point in the history
  4. Fix OTEL_BSP_MAX_CONCURRENT_EXPORTS name and value

    Per PR feedback, the default should match the previous behavior of 1
    batch at a time.
    jwilm committed May 11, 2022
    Configuration menu
    Copy the full SHA
    790609e View commit details
    Browse the repository at this point in the history
  5. Fix remaining TODOs

    This finishes the remaining TODOs on the concurrent-exports branch. The
    major change included here adds shutdown functionality to the jaeger
    exporter which ensures the exporter has finished its tasks before
    exiting.
    jwilm committed May 11, 2022
    Configuration menu
    Copy the full SHA
    c988229 View commit details
    Browse the repository at this point in the history
  6. Restore lint.sh script

    This was erroneously committed.
    jwilm committed May 11, 2022
    Configuration menu
    Copy the full SHA
    fec6c9f View commit details
    Browse the repository at this point in the history
  7. Make max concurrent exports env configurable

    OTEL_BSP_MAX_CONCURRENT_EXPORTS may now be specified in the environment
    to configure the number of max concurrent exports. This configurable now
    has parity with the other options of the span_processor.
    jwilm committed May 11, 2022
    Configuration menu
    Copy the full SHA
    b647016 View commit details
    Browse the repository at this point in the history