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

[GEN-2012]: Add TLS support for Jaeger (v1 & v2) #2021

Merged
merged 25 commits into from
Dec 19, 2024

Conversation

BenElferink
Copy link
Contributor

@BenElferink BenElferink commented Dec 18, 2024

This pull request includes several changes to enhance Jaeger configuration, improve TLS support, and update the frontend for dynamic field handling. The most important changes are summarized below:

Jaeger Configuration Enhancements:

  • common/config/jaeger.go: Added new constants JaegerTlsKey and JaegerCaPemKey, and updated error messages for better clarity.
  • common/config/jaeger.go: Modified ModifyConfig function to support both secure and insecure connections, and added validation for TLS configuration.
  • common/config/utils.go: Added parseEncryptedOtlpGrpcUrl function to handle and validate encrypted OTLP gRPC URLs.

Frontend Updates:

Documentation Updates:

These changes collectively improve the configuration flexibility and security of Jaeger integrations, while also enhancing the frontend user experience.

@BenElferink BenElferink requested a review from blumamir December 18, 2024 13:39
Copy link

Support Jaeger Over TLS

docs/backends/jaeger.mdx Outdated Show resolved Hide resolved
destinations/data/jaeger.yaml Outdated Show resolved Hide resolved
destinations/data/jaeger.yaml Outdated Show resolved Hide resolved
common/config/jaeger.go Outdated Show resolved Hide resolved
@BenElferink
Copy link
Contributor Author

BenElferink commented Dec 18, 2024

@blumamir I made all the necessary changes 😄
Also updated our internal doc: https://github.com/odigos-io/odigos-internal/pull/2

@BenElferink BenElferink requested a review from blumamir December 18, 2024 18:37
Copy link
Collaborator

@blumamir blumamir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @BenElferink

This looks great, added few nits

common/config/utils.go Outdated Show resolved Hide resolved
common/config/jaeger.go Outdated Show resolved Hide resolved
destinations/data/jaeger.yaml Outdated Show resolved Hide resolved
The endpoint format is `host:port`.
- host is required
- port is optional and defaults to the default OTLP gRPC port `4317`.
- **JAEGER_URL** - Endpoint, the format is `host:port`.
Copy link
Collaborator

@blumamir blumamir Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should use the code name here JAEGER_URL or the display name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key name makes most sense to me, it's also self explanatory.

docs/backends/jaeger.mdx Outdated Show resolved Hide resolved
docs/backends/jaeger.mdx Outdated Show resolved Hide resolved
@BenElferink
Copy link
Contributor Author

@blumamir sorted the nits too 👍

@BenElferink BenElferink requested a review from blumamir December 19, 2024 09:06
@BenElferink BenElferink enabled auto-merge (squash) December 19, 2024 12:55
@BenElferink BenElferink merged commit f2af7f4 into odigos-io:main Dec 19, 2024
31 checks passed
@BenElferink BenElferink deleted the gen-2012 branch January 6, 2025 12:34
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.

2 participants