Skip to content

Conversation

xuan-cao-swi
Copy link
Contributor

@xuan-cao-swi xuan-cao-swi commented Nov 25, 2024

Description

  1. add custom metrics with otlp protocol
  2. add metrics-sdk as one of the dependencies

Current limitation (need more clarification):

  1. user have to install metrics exporter
  2. user have to decide when the metrics exporter will happen (either through periodic reader or manual)

Test (if applicable)

@xuan-cao-swi xuan-cao-swi changed the title Nh 91603 NH-91603: custom otlp metrics through otlp protocol Nov 25, 2024
@xuan-cao-swi xuan-cao-swi marked this pull request as ready for review November 25, 2024 19:05
@xuan-cao-swi xuan-cao-swi requested a review from a team as a code owner November 25, 2024 19:05
@cheempz
Copy link
Contributor

cheempz commented Nov 26, 2024

Thanks for noting the limitation/question @xuan-cao-swi! I'd like our custom distro to do both: take a dependency on the exporter so that it is installed by default (guess it's not yet part of opentelemetry-sdk and eventually will be?), and to set up a periodic reader.

Any thoughts on possible downside / risk if we went this way?

@xuan-cao-swi
Copy link
Contributor Author

I'd like our custom distro to do both: take a dependency on the exporter so that it is installed by default (guess it's not yet part of opentelemetry-sdk and eventually will be?)

Also, otlp trace exporter is not part of opentelemetry-ruby sdk. I don't think they will include any exporter in sdk; but metrics-sdk will be part of sdk eventually.

We install opentelemetry-exporter-otlp and opentelemetry-exporter-otlp-metrics by default in lambda through layer build, but not in our agent gemspec.

To have periodic reader by default, then we may discourage user to manually pull/export the metrics. If user use aggregation_temporality as :delta, then user may experience strange metrics overtime if they use periodic reader and manual export together.

@xuan-cao-swi
Copy link
Contributor Author

Actually the PeriodicMetricReader is enabled by default: https://github.com/open-telemetry/opentelemetry-ruby/blob/opentelemetry-metrics-sdk/v0.4.0/metrics_sdk/lib/opentelemetry/sdk/metrics/configuration_patch.rb#L44

User have choice to decide the duration of exporting

@cheempz
Copy link
Contributor

cheempz commented Nov 26, 2024

We install opentelemetry-exporter-otlp and opentelemetry-exporter-otlp-metrics by default in lambda through layer build, but not in our agent gemspec.

Gotcha, let's make the same change in our custom distro, which also means no need to special case this in the lambda layer build :)

@xuan-cao-swi xuan-cao-swi deleted the NH-91603 branch June 9, 2025 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants