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

Added OTEL MeterProvider in Telemetery Settings #6144

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

Conversation

shinigami-777
Copy link

@shinigami-777 shinigami-777 commented Oct 31, 2024

Which problem is this PR solving?

Description of the changes

  • This PR introduces LeveledMeterProvider in the Telemetery Settings struct . This has been propagated to other parts in the codebase.

How was this change tested?

  • I had successfully run make fmt and make lint test for testing.

Checklist

@shinigami-777 shinigami-777 requested a review from a team as a code owner October 31, 2024 20:31
@dosubot dosubot bot added the area/otel label Oct 31, 2024
@shinigami-777 shinigami-777 changed the title added meter provider in telemetery settings Added MeterProvider in Telemetery settings Oct 31, 2024
@shinigami-777 shinigami-777 changed the title Added MeterProvider in Telemetery settings Added OTEL MeterProvider in Telemetery Settings Oct 31, 2024
@mahadzaryab1
Copy link
Collaborator

mahadzaryab1 commented Oct 31, 2024

@shinigami-777 The purpose of this ticket isn't necessarily to change how the settings are being initialized but rather to pass down the meter provider from OTEL. You can break down this task as follows:

  • Add the LeveledMeterProvider to telemetry.Setting
  • Pass in the LeveledMeterProvider wherever it is available. For example, in the jaeger-v2 query server, you can pass it in here by doing s.telset.LeveledMeterProvider. If it is not available, which will be the case for anything that is specific to v1, you can pass in the noop meter provider

@shinigami-777
Copy link
Author

@mahadzaryab1 Thanks for the clarity. I have made the necessary changes.

Metrics metrics.Factory
ReportStatus func(*componentstatus.Event)
Host component.Host
LeveledMeterProvider func(_ configtelemetry.Level) metric.MeterProvider
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that we've got this field available to us - we also want to be able to use it. For example, in the query server we currently hardcode the meter provider but now we can pass it from telset.

Copy link
Author

@shinigami-777 shinigami-777 Nov 7, 2024

Choose a reason for hiding this comment

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

Passed it in the query server.

@shinigami-777
Copy link
Author

@mahadzaryab1 I have propagated the setting everywhere in the codebase.

Copy link

codecov bot commented Nov 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.49%. Comparing base (3f752c2) to head (9bc1a11).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6144      +/-   ##
==========================================
+ Coverage   96.47%   96.49%   +0.01%     
==========================================
  Files         354      354              
  Lines       20126    20127       +1     
==========================================
+ Hits        19417    19421       +4     
+ Misses        524      522       -2     
+ Partials      185      184       -1     
Flag Coverage Δ
badger_v1 8.31% <ø> (ø)
badger_v2 1.67% <ø> (ø)
cassandra-4.x-v1 14.39% <ø> (ø)
cassandra-4.x-v2 1.61% <ø> (ø)
cassandra-5.x-v1 14.39% <ø> (ø)
cassandra-5.x-v2 1.61% <ø> (ø)
elasticsearch-6.x-v1 18.60% <ø> (+<0.01%) ⬆️
elasticsearch-7.x-v1 18.68% <ø> (+<0.01%) ⬆️
elasticsearch-8.x-v1 18.84% <ø> (-0.01%) ⬇️
elasticsearch-8.x-v2 1.67% <ø> (+0.01%) ⬆️
grpc_v1 9.47% <ø> (-0.01%) ⬇️
grpc_v2 7.00% <ø> (ø)
kafka-v1 8.88% <ø> (ø)
kafka-v2 1.67% <ø> (ø)
memory_v2 1.67% <ø> (ø)
opensearch-1.x-v1 18.72% <ø> (-0.01%) ⬇️
opensearch-2.x-v1 18.73% <ø> (ø)
opensearch-2.x-v2 1.66% <ø> (-0.02%) ⬇️
tailsampling-processor 0.46% <ø> (ø)
unittests 95.40% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Successfully merging this pull request may close these issues.

3 participants