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

Make client auth extension public and implement component.Component #814

Merged
merged 5 commits into from
Mar 18, 2024

Conversation

damemi
Copy link
Contributor

@damemi damemi commented Mar 6, 2024

Working on contributing this upstream (open-telemetry/opentelemetry-collector-contrib#31518), we will need to export some of the functions like CreateExtension and CreateDefaultConfig. This also implements the component.Component interface so the extension can be created without calling google.FindDefaultCredentials(), which can result in an immediate error if the credentials are not found. This enables lifecycle tests upstream.

@damemi damemi requested a review from a team as a code owner March 6, 2024 18:22
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

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

Project coverage is 60.07%. Comparing base (e16634a) to head (842371d).
Report is 4 commits behind head on main.

Files Patch % Lines
extension/googleclientauthextension/factory.go 80.00% 3 Missing ⚠️
extension/googleclientauthextension/grpc.go 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #814      +/-   ##
==========================================
+ Coverage   59.79%   60.07%   +0.28%     
==========================================
  Files          56       56              
  Lines        5743     5896     +153     
==========================================
+ Hits         3434     3542     +108     
- Misses       2159     2202      +43     
- Partials      150      152       +2     

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

@dashpole
Copy link
Contributor

dashpole commented Mar 6, 2024

Given how small this library is, should the entire thing just live upstream?

@damemi
Copy link
Contributor Author

damemi commented Mar 7, 2024

Given how small this library is, should the entire thing just live upstream?

I'm thinking it's best to do it the same way we do with our exporters, where the code lives downstream so we have more control over testing and updates. We can always follow up with moving the rest of the code upstream if we want.

@damemi damemi force-pushed the make-extension-public branch from c831eea to 09a443b Compare March 15, 2024 13:30
@damemi damemi changed the title Make client auth extension public and add CredentialsFile config path Make client auth extension public and implement component.Component Mar 15, 2024
@damemi
Copy link
Contributor Author

damemi commented Mar 15, 2024

@dashpole updated this like we talked about offline, ready for review now

@damemi damemi merged commit 2be0414 into GoogleCloudPlatform:main Mar 18, 2024
32 checks passed
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.

2 participants