-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Expose functions for registering gRPC(-gateway) services #2395
Comments
#2411 implements this as described. If there are no objections in principle, I'll open that up for review. |
It is not clear why you need to expose these functions. Are you implementing a receiver in the Collector? If you are implementing an OTLP receiver in your codebase that is not the Collector then you should generate your own OTLP gRPC/Protobuf messages from https://github.com/open-telemetry/opentelemetry-proto/tree/main/opentelemetry/proto |
@tigrannajaryan no, not a receiver in the Collector. Rather. We would like to embed the Collector inside another existing program (apm-server). The goal is to add support for receiving OTLP directly in apm-server.
Yes, that is another option. We would prefer to reuse code if possible. If we generate independent code from the protobuf definitions, anything we develop around that cannot easily be reused in opentelemetry-collector. Maybe there's never anything to share back, but I thought the small amount of code involved in implementing this (#2411) would be worth keeping the possibility open. |
Making these functions exported increases our API surface and is not something we want to do, no matter how small the increase. We are constantly looking for ways to keep our public API small. So, unfortunately, the answer is no. You may be able to use https://github.com/open-telemetry/opentelemetry-proto-go once it is ready. I am going to close this, please feel free to reopen if you have additional arguments. |
No worries, thanks for the discussion. Is the intention to have the collector use opentelemetry-proto-go at some point? (If this is documented somewhere I'd be happy to read up - couldn't find any issues mentioning it.) |
…ry#2395) * Bump go.uber.org/multierr from 1.8.0 to 1.9.0 in /tests Bumps [go.uber.org/multierr](https://github.com/uber-go/multierr) from 1.8.0 to 1.9.0. - [Release notes](https://github.com/uber-go/multierr/releases) - [Changelog](https://github.com/uber-go/multierr/blob/master/CHANGELOG.md) - [Commits](uber-go/multierr@v1.8.0...v1.9.0) --- updated-dependencies: - dependency-name: go.uber.org/multierr dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * tidy Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Antoine Toulme <[email protected]>
Is your feature request related to a problem? Please describe.
I develop Elastic APM Server, for which there is an exporter in opentelemetry-collector-contrib.
We would like to support receiving OTLP directly in Elastic APM Server, cutting out the need to translate to our own protocol. For basic use cases this would enable users to send telemetry data directly to Elastic APM Server, reserving opentelemetry-collector for situations where additional preprocessing is needed, or for sending to multiple backends.
Furthermore, we would like to receive OTLP data on the same port as used by Elastic APM agents, which would necessitate using a shared gRPC server and grpc-gateway mux. This means we cannot use the existing
otlpreceiver
API, which owns the gRPC/HTTP servers/ports. We would call theRegisterTraceServiceServer
,RegisterTraceServiceHandlerServer
functions directly, but they are hidden inside an internal package.Describe the solution you'd like
Add exported functions to
receiver/otlpreceiver
:(I'm more than happy to send a PR to this effect, if deemed reasonable.)
Describe alternatives you've considered
internal/data/protogen
out of internal. This would expose too much about the internal details.The text was updated successfully, but these errors were encountered: