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

Add SDK Log health metrics #1921

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

JonasKunz
Copy link
Contributor

@JonasKunz JonasKunz commented Feb 19, 2025

Follow up of #1631 adding SDK health metrics for logs.

Changes

Adds:

  • A metric to count the number of logs generated submitted to the SDK (only enabled loggers count)
  • Count metrics for LogRecordProcessors and LogRecordExporters to track data loss

The processor and exporter metrics are very similar to the span-ones in #1631. In fact, they are pretty much a copy-paste.
So I think with the concrete examples at hand now, it would be a good time to revisit the decision and reasoning whether to keep the metrics separated per signal or combine them with and differentiate via an attribute (e.g. otel.signal.kind=span,log,metric).

The detailed reasoning for separation can be found in this comment.

I still think that reasoning holds for processors: Log and Span Processors do have the same name, but work differently (e.g. span processor has multiple callbacks). Therefore it makes sense to have different namespaces for their metrics to not make things ugly in the future when new, span or log specific metrics need to be added or the processor specs diverge further. This already shows in the fact that metrics do not have processors, but MetricReaders.

For the exporters I'm not so sure anymore: The exporters are separated in the spec, but it is in fact mostly copy-pasted and I think they are unlikely to diverge. So if we wanted to reduce the size of the spec here and ease some queries (e.g. "are there errors in any exporter?"), we could think of combining them.

Looking forward to hearing your opinions here!

Merge requirement checklist

@github-actions github-actions bot added the enhancement New feature or request label Feb 19, 2025
@JonasKunz JonasKunz marked this pull request as ready for review February 19, 2025 11:35
@JonasKunz JonasKunz requested review from a team as code owners February 19, 2025 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

1 participant