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

Make HTTP as the default connection option for Tempo and add disclaimer for gRPC #3596

Open
mathnogueira opened this issue Feb 6, 2024 · 6 comments
Labels
design enhancement New feature or request

Comments

@mathnogueira
Copy link
Contributor

Describe the enhancement you'd like to see
When Tempo is deployed in a microservice infrastructure, it doesn't ship with the gRPC endpoint we use for getting a trace by ID. We are using an internal API to fetch it over gRPC.

The solution for that would be changing the default option from gRPC to HTTP in the datastore screen add a disclaimer to the gRPC option and explaining to the user that depending on how Tempo is deployed, it might not have the required endpoint Tracetest requires. They should prefer an HTTP connection instead.

image

@mathnogueira mathnogueira added enhancement New feature or request triage requires triage and removed triage requires triage labels Feb 6, 2024
@jorgeepc jorgeepc added the design label Feb 6, 2024
@jorgeepc
Copy link
Contributor

jorgeepc commented Feb 6, 2024

Hey @olha23 could you take a look at this ticket and help us with the disclaimer design? Thank you!

@mathnogueira
Copy link
Contributor Author

Thomas added a very pertinent comment:

I would recommend not providing GRPC at all. While it'll appear to work in single-binary mode, that GRPC endpoint will only return traces that are still in memory. It won't pick up anything that's already written to disk. Which could cause some very difficult to debug issues for people

Basically, if tempo gets restarted, Tracetest will not be able to fetch traces. We probably need to remove the gRPC support for tempo and tell users to update the tracing backend.

I think we can add a deprecation note to it.

@jorgeepc
Copy link
Contributor

jorgeepc commented Feb 6, 2024

Interesting @mathnogueira. What do you think @kdhamric?

@kdhamric
Copy link
Collaborator

kdhamric commented Feb 6, 2024

Wow. Yes, we need to drop it.

By the deprecation notice, are you saying note it in the screen for a period of time, then remove the option? @mathnogueira

@olha23
Copy link

olha23 commented Feb 7, 2024

Consider removing the gRPC support for Tempo entirely i think we need provide users with clear instructions on updating their tracing backend to support HTTP connections instead and add a deprecation note to the gRPC option, maybe something like this: "Switch to HTTP for trace retrieval in Tempo! gRPC support is being phased out. Update your backend settings now to avoid interruptions. Thanks!"

@mathnogueira
Copy link
Contributor Author

Yes @kdhamric . I think we can keep the option for a while with some kind of banner to tell users about the the deprecation and how they can migrate to HTTP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants