Configure Jaeger receiver and exporter by flags #2241
Conversation
|
I have moved TLS test to OTEL collector open-telemetry/opentelemetry-collector#962 |
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
| "--collector.grpc.tls.cert=cacert.crt", | ||
| "--collector.grpc.tls.key=keycert.crt", | ||
| "--collector.http-server.host-port=host2:22", | ||
| "--reporter.grpc.host-port=coll:33", |
There was a problem hiding this comment.
Might be missing something, but why would these reporter flags be defined on the collector? They are normally only supported to an agent.
I'm guessing its for when the collector maybe forwarding data to another collector using those reporter (or exporter) details - but think it may need some explanation of why these exporter connection details are being used for the receiver/remote sampling endpoint?
There was a problem hiding this comment.
Collector and agent use OTEL Jaeger receiver. Jaeger receiver exposes HTTP sampling endpoint. It can be configured to get the strategies from a remote server via gRPC. Legacy collector was using the reporter (and its flags) to get the data from the collector.
I can update the test and move the flags to agent test, but it really does not matter in the end.
| "--reporter.grpc.tls.cert=cert.pem", | ||
| "--reporter.grpc.tls.key=key.key", | ||
| }, | ||
| zipkinHostPort: "localhost:55", |
There was a problem hiding this comment.
Why is the zipkin host/port being provided separately - couldn't it be taken from the --collector.zipkin.host-port flag?
There was a problem hiding this comment.
Because we parse the flag in main and enable the receiver only in the default config.
The zipkin receiver is different to Jaeger components, we do not wrap and we enable it only in the default config. I will create a separate issue for this.
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
| if v.IsSet(thriftCompactHostPort) { | ||
| cfg.Protocols["thrift_compact"] = &receiver.SecureReceiverSettings{ | ||
| ReceiverSettings: configmodels.ReceiverSettings{ | ||
| // TODO OTEL does not expose number of workers and queue length |
There was a problem hiding this comment.
Wouldn't these be handled by the processors, e.g. queued_retry? So possibly they are not going to be supported by flags going forward - or possibly new flags that can be used to configure the processors?
There was a problem hiding this comment.
the workers and queue length are used on agent implementation it's agnostic to the queued_retry processor.
| } | ||
| if cOpts.TLS.CertPath != "" && cOpts.TLS.KeyPath != "" { | ||
| cfg.Protocols["grpc"].TLSCredentials = &receiver.TLSCredentials{ | ||
| // TODO client-ca is missing in OTEL |
There was a problem hiding this comment.
Has an issue been created to track this?
There was a problem hiding this comment.
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2241 +/- ##
=======================================
Coverage 96.18% 96.18%
=======================================
Files 219 219
Lines 10640 10648 +8
=======================================
+ Hits 10234 10242 +8
+ Misses 351 350 -1
- Partials 55 56 +1
Continue to review full report at Codecov.
|
This PR makes Jaeger receiver and exporter fully configurable by flags.
Resolves #2181
In a nutshell:
AddFlags()AddComponentFlags()that adds flags for all components except storage exporters. This is used in every main to add flags to cobraAddFlagsfrom legacy jaeger packages are split intoAddOTELFlagsto make sure we add only flags that are neededTodos: