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

Support Otlp Tracing's GRPC port from service connections #41333

Conversation

eddumelendez
Copy link
Contributor

Otlp Tracing's exporter is configured using Transport. Current support for service connections read the mapped port for HTTP transport 4318. This commits adds support to read port for GRPC transport 4317.

Otlp Tracing's exporter is configured using `Transport`. Current support
for service connections read the mapped port for HTTP transport `4318`.
This commits adds support to read port for GRPC transport `4317`.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 5, 2024
@wilkinsona
Copy link
Member

wilkinsona commented Jul 5, 2024

Thanks for the PR, @eddumelendez. I'm not sure what the answer is, but this doesn't feel quite right to me. I don't think the connection details should have a separate method for the GRPC endpoint. Ideally, the existing getUrl() method would return the URL that's appropriate for the configured transport but I'm not yet sure how we could achieve that.

@eddumelendez
Copy link
Contributor Author

Hi @wilkinsona, yes, I agree. I think that's the only way I was able to achieve it. Looking forward for any hint to update the PR.

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jul 8, 2024
@philwebb
Copy link
Member

We discussed this today and we think we might be able to pass the Transport to a OtlpTracingConnectionDetails.getUrl(...) method. We can experiment with this before we merge the PR.

@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Jul 10, 2024
@philwebb philwebb added this to the 3.4.x milestone Jul 10, 2024
@philwebb philwebb added the for: merge-with-amendments Needs some changes when we merge label Jul 10, 2024
@wilkinsona wilkinsona self-assigned this Jul 11, 2024
wilkinsona pushed a commit to wilkinsona/spring-boot that referenced this pull request Jul 11, 2024
Otlp Tracing's exporter is configured using `Transport`. Current support
for service connections read the mapped port for HTTP transport `4318`.
This commits adds support to read port for GRPC transport `4317`.

See spring-projectsgh-41333
wilkinsona added a commit to wilkinsona/spring-boot that referenced this pull request Jul 11, 2024
@wilkinsona wilkinsona added the status: on-hold We can't start working on this issue yet label Jul 11, 2024
@wilkinsona
Copy link
Member

Placing on hold as we'd like to spend a bit of time thinking through our gRPC support across logs, metrics, and tracing.

@wilkinsona wilkinsona removed the status: on-hold We can't start working on this issue yet label Aug 20, 2024
@wilkinsona wilkinsona removed their assignment Aug 20, 2024
mhalbritter pushed a commit that referenced this pull request Sep 6, 2024
Otlp Tracing's exporter is configured using Transport. Current support
for service connections read the mapped port for HTTP transport 4318.
This commits adds support to read port for GRPC transport 4317.

See gh-41333
@mhalbritter
Copy link
Contributor

Thanks!

I've modified OtlpTracingConnectionDetails to add a String getUrl(Transport transport) method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants