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

[Bug]: PeriodicReader uses sync::Mutex. #1507

Open
bIgBV opened this issue Feb 1, 2024 · 0 comments
Open

[Bug]: PeriodicReader uses sync::Mutex. #1507

bIgBV opened this issue Feb 1, 2024 · 0 comments
Assignees
Labels
A-metrics Area: issues related to metrics bug Something isn't working shutdown&runtime

Comments

@bIgBV
Copy link
Contributor

bIgBV commented Feb 1, 2024

What happened?

The PeriodicReader internally uses a sync::Mutex to synchronize on the PeriodicReaderInner type:

https://github.com/open-telemetry/opentelemetry-rust/blob/d19187db46ec445143ecdc0271794f71dce3055d/opentelemetry-sdk/src/metrics/periodic_reader.rs#L201C1-L205C2

This can possibly cause deadlocks if the mutex is held across an .await point(https://docs.rs/tokio/latest/tokio/sync/struct.Mutex.html#which-kind-of-mutex-should-you-use). And it looks like there is at least one such case of a user reporting this on the #otel-rust slack channel.

API Version

0.21

SDK Version

0.21.2

What Exporters are you seeing the problem on?

N/A

Relevant log output

$ RUST_LOG=debug cargo run -- --poll-interval 30s
[2024-01-25T19:25:41Z INFO  nomad_otel_metrics_scraper] Polling http://localhost:4646/ every 30s
{
  "resourceMetrics": {
    "resource": {
      "attributes": [
        {
          "key": "telemetry.sdk.version",
          "value": {
            "stringValue": "0.21.2"
          }
        },
        {
          "key": "telemetry.sdk.language",
          "value": {
            "stringValue": "rust"
          }
        },
        {
          "key": "telemetry.sdk.name",
          "value": {
            "stringValue": "opentelemetry"
          }
        },
        {
          "key": "service.name",
          "value": {
            "stringValue": "unknown_service"
          }
        }
      ]
    },
    "scopeMetrics": []
  }
}
^C[2024-01-25T19:25:43Z INFO  nomad_otel_metrics_scraper] Provider, as we know it. MeterProvider {
        pipes: Pipelines(
            [
                Pipeline,
            ],
        ),
        meters: Mutex {
            data: {
                InstrumentationLibrary {
                    name: "nomad_metrics",
                    version: None,
                    schema_url: None,
                    attributes: [],
                }: Meter {
                    scope: InstrumentationLibrary {
                        name: "nomad_metrics",
                        version: None,
                        schema_url: None,
                        attributes: [],
                    },
                },
            },
            poisoned: false,
            ..
        },
        is_shutdown: false,
    }
[2024-01-25T19:25:43Z INFO  nomad_otel_metrics_scraper] Flushing metrics.
@bIgBV bIgBV added bug Something isn't working triage:todo Needs to be traiged. labels Feb 1, 2024
@lalitb lalitb added this to the Metrics SDK Stable milestone Feb 1, 2024
@cijothomas cijothomas added A-metrics Area: issues related to metrics and removed triage:todo Needs to be traiged. labels Feb 10, 2024
@cijothomas cijothomas self-assigned this Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-metrics Area: issues related to metrics bug Something isn't working shutdown&runtime
Projects
None yet
Development

No branches or pull requests

3 participants