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

config: NewSDK can return valid MeterProvider #4804

Merged

Conversation

codeboten
Copy link
Contributor

@codeboten codeboten commented Jan 11, 2024

Follow up to #4741, does the same but for metric signal.

Fixes #4371

Will rebase once 4741 is merged

@codeboten codeboten changed the title config: NewSDK can return valid MeterProvider [WIP] config: NewSDK can return valid MeterProvider Jan 11, 2024
Copy link

codecov bot commented Jan 11, 2024

Codecov Report

Attention: Patch coverage is 84.72222% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 62.1%. Comparing base (7a9e861) to head (997c85e).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #4804     +/-   ##
=======================================
+ Coverage   61.8%   62.1%   +0.2%     
=======================================
  Files        186     186             
  Lines      11388   11527    +139     
=======================================
+ Hits        7049    7166    +117     
- Misses      4137    4149     +12     
- Partials     202     212     +10     
Files Coverage Δ
config/config.go 85.0% <33.3%> (-4.5%) ⬇️
config/metric.go 85.9% <85.8%> (-14.1%) ⬇️

@codeboten codeboten force-pushed the codeboten/add-meter-provider-sdk branch 2 times, most recently from 905a8ac to 792c424 Compare January 22, 2024 21:46
@codeboten codeboten marked this pull request as ready for review January 22, 2024 21:55
@codeboten codeboten changed the title [WIP] config: NewSDK can return valid MeterProvider config: NewSDK can return valid MeterProvider Jan 22, 2024
@pellared
Copy link
Member

pellared commented Feb 19, 2024

@codeboten Can you please update the branch (resolve conflicts)? I will do my best to review the PR this week.

@MadVikingGod MadVikingGod added the area: config Related to config functionality label Feb 20, 2024
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

I think that one unit test should be added. Other than that LGTM.

config/metric.go Outdated Show resolved Hide resolved
Copy link

linux-foundation-easycla bot commented Mar 4, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: codeboten / name: Alex Boten (997c85e)

@codeboten codeboten force-pushed the codeboten/add-meter-provider-sdk branch from 2d2a5b1 to 8e92da2 Compare March 4, 2024 18:52
@codeboten codeboten force-pushed the codeboten/add-meter-provider-sdk branch from 8e92da2 to 5508d92 Compare March 4, 2024 20:57
Comment on lines +226 to +228
// TODO: add support for constant label filter
// otelprom.WithResourceAsConstantLabels(attribute.NewDenyKeysFilter()),
// )
Copy link
Member

Choose a reason for hiding this comment

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

Can you create an issue for tracking and reference it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done #5381

Follow up to open-telemetry#4741, does the same but for metric signal.

Fixes open-telemetry#4371

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten codeboten force-pushed the codeboten/add-meter-provider-sdk branch from 5508d92 to 997c85e Compare April 12, 2024 17:12
@pellared pellared merged commit 07d8068 into open-telemetry:main Apr 12, 2024
22 checks passed
@codeboten codeboten deleted the codeboten/add-meter-provider-sdk branch April 12, 2024 17:36
scorpionknifes added a commit to scorpionknifes/opentelemetry-go-contrib that referenced this pull request Apr 24, 2024
scorpionknifes pushed a commit to scorpionknifes/opentelemetry-go-contrib that referenced this pull request Apr 24, 2024
@MrAlias MrAlias added this to the v1.26.0 milestone Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: config Related to config functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

config: Create SDK from model
6 participants