Skip to content

fix!: align with the spec for metrics sdk and metrics otlp exporter - #2301

Open
xuan-cao-swi wants to merge 6 commits into
open-telemetry:mainfrom
xuan-cao-swi:metrics-exporter-spec
Open

fix!: align with the spec for metrics sdk and metrics otlp exporter#2301
xuan-cao-swi wants to merge 6 commits into
open-telemetry:mainfrom
xuan-cao-swi:metrics-exporter-spec

Conversation

@xuan-cao-swi

Copy link
Copy Markdown
Contributor

Description

  1. periodic metrics reader with console exporter default 10s export interval - spec
  2. follow the python metrics otlp implementation that shutdown should stop export - py

@mwear mwear left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but there are some rubocop failures that need to be addressed.

@simi

simi commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Not sure if related directly to this change, but shouldn't we synchronize shutdown with the export mutex or re-check @shutdown inside the synchronized section? It seems there's lifecycle race now.

@kaylareopelle kaylareopelle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Do we want any tests to make sure we don't drift away from these changes?

@kaylareopelle

Copy link
Copy Markdown
Contributor

Not sure if related directly to this change, but shouldn't we synchronize shutdown with the export mutex or re-check @shutdown inside the synchronized section? It seems there's lifecycle race now.

That's a good question, @simi. It might be a separate change to think through. If I'm understanding your question, we don't synchronize shutdown checks in our other exporters. The main OTLP (Traces) exporter doesn't use a mutex at all. I think we should look into this more. If we find a solution, it should be applied to traces, metrics, and logs.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants