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

Generalize concrete implementation registration and lookup. #967

Closed
tschroed opened this issue May 15, 2017 · 7 comments
Closed

Generalize concrete implementation registration and lookup. #967

tschroed opened this issue May 15, 2017 · 7 comments
Labels
enhancement Feature requests. Not bugs or questions.
Milestone

Comments

@tschroed
Copy link
Contributor

Tracing, stats, and logging should have dynamic registration for runtime lookup of specific implementations.

For example, right now there is a lightstep and a zipkin tracing implementation. These are selected using config values and if/else in MainImpl::initializeTracers.

Sometimes it's undesirable to even attempt to link against specific implementations. For example, Lightstep introduces a symbol collision with existing google3 code which we want to link in.

Instead we should have a dynamic registration mechanism similar to RegisterNetworkFilterConfigFactory that allows us to selectively link in dependencies.

@htuch
Copy link
Member

htuch commented May 15, 2017

Do we want to call this static registration to distinguish from fully dynamic registration (e.g. using ELF loadable objects?).

@tschroed
Copy link
Contributor Author

I'm name agnostic. We could even just call it "registration".

@mattklein123
Copy link
Member

+1 to this, and also doing the bazel work to make it easy to compile things in and out. The static registration can be modeled after the existing filter stuff.

@htuch
Copy link
Member

htuch commented May 15, 2017

Should we use reverse DNS naming notation? Also see https://github.com/lyft/envoy-api/issues/34.

@tschroed
Copy link
Contributor Author

Works for me.

Slightly more crazy: we could have a single registry for all of this stuff to make it readily extensible to new classes of things. However, retrieval probably ends up involving a static_cast which makes me a bit uncomfortable.

@mattklein123
Copy link
Member

I think you mean dynamic_cast, but the single registry is an interesting idea and might remove duplication. Don't feel strongly either way.

@mattklein123 mattklein123 added the enhancement Feature requests. Not bugs or questions. label May 16, 2017
mrice32 added a commit to mrice32/envoy that referenced this issue May 18, 2017
@mattklein123 mattklein123 modified the milestone: 1.4.0 May 19, 2017
htuch pushed a commit that referenced this issue May 24, 2017
Added a static registration mechanism (following the pattern of filters mentioned in the comments on #967) for http tracers. This will likely be expanded to a more general registration mechanism in future work. This PR is part of the work towards solving #967.

This commit deprecates the existing HttpFilterConfigFactory filter API in favor of NamedHttpFilterConfigFactory.
@mattklein123
Copy link
Member

@tschroed I'm going to close this as done. I think we have general agreement on approach here. We can add new registration for things like stats as needed.

htuch pushed a commit that referenced this issue Sep 5, 2017
This is part of a larger effort to make various implementations more flexible by allowing proprietary components to be statically registered with Envoy needing only to be linked to the binary at build time and configured at runtime #967 .

The user-visible configuration changes do involve a few deprecations: statsd_udp_ip_address and statsd_tcp_cluster_name will be deprecated, and their equivalents will be moved to a new stats_sinks array where the configurations for any statically registered sink will exist. As a part of this change, all integration tests were moved to the new configuration format, but a new configuration test was added to ensure that the deprecated format continues to work until it is removed in 1.5.0.
rshriram pushed a commit to rshriram/envoy that referenced this issue Oct 30, 2018
Automatic merge from submit-queue.

Extract the subject field of a JWT

Extract the subject field (i.e., sub) of a JWT, which is used during the authentication.
To run the tests: bazel test //src/envoy/auth:jwt_test

**What this PR does / why we need it**:
Extract the subject field (i.e., sub) of a JWT, which is used during the authentication.
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes envoyproxy#966

**Special notes for your reviewer**:

**Release note**:

```release-note
```
jpsim pushed a commit that referenced this issue Nov 28, 2022
Description: Changes filter instantiation to be a per-stream basis, across the core, bridge, and iOS platform layers. Also improves usage of CFBridge* functions to be better in line with their intent.
Risk Level: Moderate
Testing: Local end-to-end

Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this issue Nov 29, 2022
Description: Changes filter instantiation to be a per-stream basis, across the core, bridge, and iOS platform layers. Also improves usage of CFBridge* functions to be better in line with their intent.
Risk Level: Moderate
Testing: Local end-to-end

Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions.
Projects
None yet
Development

No branches or pull requests

3 participants