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

PeriodicExportingMetricReader tries to export when there's no data #5234

Open
alexmojaki opened this issue Dec 4, 2024 · 3 comments
Open
Assignees
Labels
bug Something isn't working pkg:sdk-metrics priority:p3 Bugs which cause problems in user apps not related to correctness (performance, memory use, etc)

Comments

@alexmojaki
Copy link

alexmojaki commented Dec 4, 2024

What happened?

Create a PeriodicExportingMetricReader wrapping an OTLPMetricExporter. Flush the reader without recording any metrics. It will send an effectively empty export request, which is wasteful for both the client and receiver. We host a receiving backend and logged an error when receiving requests like this, and took a while to investigate because I didn't expect that the client would actually send such requests.

I imagine PeriodicExportingMetricReader should be the one to skip exporting/reading when there's nothing to export, but this could maybe also be done in OTLPMetricExporter and other exporters. The console exporter naturally does nothing because it iterates through things to export, so the loop is empty.

OpenTelemetry Setup Code

import {NodeSDK} from "@opentelemetry/sdk-node";
import {PeriodicExportingMetricReader} from "@opentelemetry/sdk-metrics";
import {OTLPMetricExporter} from "@opentelemetry/exporter-metrics-otlp-proto";

const exporter = new OTLPMetricExporter();
const metricReader = new PeriodicExportingMetricReader({exporter});
const sdk = new NodeSDK({metricReader});
sdk.start();
metricReader.forceFlush().then(() => console.log('Metrics flushed'));

package.json

{
  "type": "module",
  "dependencies": {
    "@opentelemetry/exporter-metrics-otlp-proto": "^0.56.0",
    "@opentelemetry/sdk-metrics": "^1.29.0",
    "@opentelemetry/sdk-node": "^0.56.0"
  }
}

Relevant log output

Sample of the export, i.e. just a bundle of resource attributes with no actual data.

resource_metrics {
  resource {
    attributes {
      key: "service.name"
      value {
        string_value: "unknown_service:node"
      }
    }
    attributes {
      key: "telemetry.sdk.language"
      value {
        string_value: "nodejs"
      }
    }
    attributes {
      key: "telemetry.sdk.name"
      value {
        string_value: "opentelemetry"
      }
    }
    attributes {
      key: "telemetry.sdk.version"
      value {
        string_value: "1.29.0"
      }
    }
  }
}

Operating System and Version

No response

Runtime and Version

No response

@alexmojaki alexmojaki added bug Something isn't working triage labels Dec 4, 2024
@pichlermarc pichlermarc added priority:p3 Bugs which cause problems in user apps not related to correctness (performance, memory use, etc) pkg:sdk-metrics labels Dec 4, 2024
@pichlermarc
Copy link
Member

Hi @alexmojaki, thanks for reaching out.

Jep, that's not optimal, we should change that. I don't think the spec define what to do in that case so we can choose our own way of handling it. Dropping it as early as possible in the PeriodicExportingMetricReader sounds like the most reasonable approach to me.

@pichlermarc pichlermarc removed the triage label Dec 4, 2024
@JacksonWeber
Copy link
Contributor

@pichlermarc Happy to take a look at this one.

@pichlermarc
Copy link
Member

thank you @JacksonWeber, it's yours 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:sdk-metrics priority:p3 Bugs which cause problems in user apps not related to correctness (performance, memory use, etc)
Projects
None yet
Development

No branches or pull requests

3 participants