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.

Sign up for GitHub

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

Jump to bottom

Plan to revert the revert PR#3276 #3282

Closed
bogdandrutu opened this issue May 24, 2021 · 7 comments
Closed

Plan to revert the revert PR#3276 #3282

bogdandrutu opened this issue May 24, 2021 · 7 comments
Assignees
@jpkrohling

Comments

@bogdandrutu
Copy link
Member

In order to make this possible let's follow the next plan:

  • Change all components to call ToDialOptions/ToClient in Start (no change on the APIs but only where it is called). Needs to happen in core and contrib.
  • Re-merge what is left in the reverted PR.

/cc @pavankrish123
@jpkrohling
Copy link
Member

I see that quite a few about this has been done already, with PRs open. The second part is missing but depends on the first.

This was referenced May 25, 2021
Closed
Merged
Merged
Merged
Merged
Merged
Merged
@pavankrish123
Copy link
Contributor

Once #3321 is merged - the upcoming changes will update the APIs ToClient() and ToDialOptions() to ToClient(ext map[config.ComponentID]component.Extension) and ToDialOptions(ext map[config.ComponentID]component.Extension).

@bogdandrutu The ext to these APIs we are proposing is a mandatory argument - just wanted to confirm if you are ok with it as well. Can make it optional as well.

@gramidt let me know if all the components are prepared - I will push the changes to APIs in the next PR.

@gramidt
Copy link
Member

gramidt commented May 27, 2021 •
edited
Loading

@pavankrish123 - The last component needing to be merged as part of #3507 is the Splunk SignalFX exporter #3511. Once that has been merged, contrib should be good to go.

@pavankrish123
Copy link
Contributor

Thanks @gramidt let me know. I have started preparing the changes

Pavan

@gramidt
Copy link
Member

gramidt commented May 28, 2021

@pavankrish123 - The final PR in contrib has been merged.

@pavankrish123
Copy link
Contributor

pavankrish123 commented May 28, 2021 •
edited
Loading

Thanks @gramidt waiting for #3321 to be merged. Will push the api changes after the merge

tigrannajaryan pushed a commit that referenced this issue May 28, 2021
@pavankrish123
Port client side authentication interfaces from PR #3128 (#3321)
7cd868d 
This PR ports the client authenticator interfaces from PR #3128  in a piece meal fashion. The interfaces are currently not used by any client configurations and are published for reviewing only. 

Plan is to modify ToClient and ToDialOptions apis once all the clients are prepared per #3287 (core and contrib side as well).

Link to tracking Issue:
#3287 #3282

Testing:
Unit tests, [manual test described (for only oidc)](#3128 (comment))
This was referenced May 28, 2021
Merged
Merged
tigrannajaryan pushed a commit that referenced this issue May 31, 2021
@pavankrish123
Refactored OpenCensus exporter to move connection into start for upco…
d1016eb 
…ming Auth changes. (#3339)

Updated OpenCensus for upcoming client auth extension changes

Link to tracking Issue:
#3287 #3282
tigrannajaryan pushed a commit that referenced this issue Jun 3, 2021
@pavankrish123
Updated configgrpc ToDialOptions and confighttp ToClient apis to take…
5369d7e 
… extensions configuration map (#3340)

This PR is a port of the configfrpc's ToDialOptions() and confighttp ToClient() from PR #3128  in a piece meal fashion. 

The following are the changes
- Refactored configgrpc.PerRPCAuth as extension implementing configauth.GrpcClientAuthenticator
- Plugged in extensions configuration to all the grpc based clients in the core (OTLP, OpenCensus, Jaeger, JaegerReceiver)
- Plugged in extensions configuration to all the HTTP based clients in the core (Zipkin, OTLPHTTP)

Link to tracking Issue:
 #3282 #3276

Testing:
Unit tests, [manual test described (for only oidc)](#3128 (comment))
@gramidt gramidt mentioned this issue Jun 3, 2021
Merged
@pavankrish123
Copy link
Contributor

Can close this issue now.

tigrannajaryan pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this issue Jun 3, 2021
@gramidt
Update core to latest and configure components for the Client Authent…
bb5f655 
…ication Extensions (2/2) (#3694)

This is the second phase of #3507.

**Link to tracking Issue:**
#3692 

Component changes are based on open-telemetry/opentelemetry-collector#3340.

Also related to open-telemetry/opentelemetry-collector#3282
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jpkrohling jpkrohling

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jpkrohling @pavankrish123 @gramidt @bogdandrutu