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

feat(jaeger-influxdb): forgive and handle schema://host:port #235

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

jacobmarble
Copy link
Member

This affects flag --influxdb-addr / env var INFLUXDB_ADDR. Because the demo requires an HTTP URL for write (otelcol), and a gRPC host:port for query (jaeger-influxdb), let's be flexible with the jaeger-influxdb constraints by allowing prefixes http://, https://, grpc://, grpc+tcp://, grpc+tls://.

Accepting the HTTP prefixes is "forgiving". Accepting the gRPC prefixes is treated as "normal" handling, just like host:port.

I've added unit tests this time. :) Also, tested with the Docker Compose demo.

@jacobmarble jacobmarble requested a review from philjb April 28, 2023 14:51
@jacobmarble jacobmarble requested a review from a team as a code owner April 28, 2023 14:51
This affects flag --influxdb-addr / env var INFLUXDB_ADDR. Because the
demo requires an HTTP URL for write (otelcol), and a gRPC host:port for
query (jaeger-influxdb), let's be flexible with the jaeger-influxdb
constraints by allowing prefixes http://, https://, grpc://,
grpc+tcp://, grpc+tls://.

Accepting the HTTP prefixes is "forgiving". Accepting the gRPC prefixes
is treated as "normal" handling, just like host:port.

I've added unit tests this time. :) Also, tested with the Docker
Compose demo.
@jacobmarble jacobmarble force-pushed the jgm-jaeger-influxdb-addr-parse branch from 9d75dd9 to f0e0b8e Compare April 28, 2023 17:19
@jacobmarble jacobmarble enabled auto-merge (rebase) April 28, 2023 17:19
@jacobmarble jacobmarble merged commit 8795d26 into main Apr 28, 2023
@jacobmarble jacobmarble deleted the jgm-jaeger-influxdb-addr-parse branch April 28, 2023 21:08
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.

None yet

2 participants