open-telemetry / open-telemetry/opentelemetry-python

[bug] PeriodicExportingMetricReader missing validation for export_timeout_millis <= 0

Open
#5,655 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Python
Stars
2.6k
Forks
1k
Avg merge
4d 15h
Merged PRs (30d)
19

Description

Describe your environment

OS: macOS / Linux
Python version: 3.10+
SDK version: main

What happened?

In opentelemetry-sdk (opentelemetry.sdk.metrics.export.PeriodicExportingMetricReader), export_interval_millis validates that its value is positive and raises a ValueError if export_interval_millis <= 0.

However, export_timeout_millis lacks validation for non-positive or negative values (<= 0). Initializing PeriodicExportingMetricReader(ConsoleMetricExporter(), export_timeout_millis=-1) or export_timeout_millis=0 is accepted without raising a ValueError.

Since _ticker() continuously calls self.collect(timeout_millis=self._export_timeout_millis), an unvalidated non-positive timeout value is passed into metric collection on every tick.

Steps to Reproduce

from opentelemetry.sdk.metrics.export import (
ConsoleMetricExporter,
PeriodicExportingMetricReader,
)

1. export_interval_millis correctly validates <= 0:

try:
PeriodicExportingMetricReader(
ConsoleMetricExporter(), export_interval_millis=0
)
except ValueError as e:
print("export_interval_millis validation working:", e)

2. export_timeout_millis lacks validation:

reader = PeriodicExportingMetricReader(
ConsoleMetricExporter(), export_timeout_millis=-1
)
print("Accepted without ValueError! actual=", reader._export_timeout_millis)

Expected Result

Initializing PeriodicExportingMetricReader with export_timeout_millis <= 0 should raise a ValueError (e.g. f"timeout value {self._export_timeout_millis} is invalid and needs to be larger than zero."), consistent with export_interval_millis.

Actual Result

Accepted without raising a ValueError.

Additional context

Location: opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py around lines 451-482.

I am opening a PR for this fix :))

Would you like to implement a fix?

Yes

Tip

React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it. Learn more here.

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

Read opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/init.py around lines 451-482, starting with PeriodicExportingMetricReader and its existing export_interval_millis validation. Reproduce the constructor calls from the issue for zero and negative export_timeout_millis, then verify that both are rejected consistently with the interval validation.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
observability-sre
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.