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

Return the cluster that was used in the query execution as response metadata #465

Open
0x726f opened this issue Sep 10, 2024 · 7 comments
Open

Comments

@0x726f
Copy link

0x726f commented Sep 10, 2024

We run several Trino clusters behind the gateway and would like the ability to store which cluster was used in query execution for each specific request on the client side.

Our client side stores a whole bunch of more useful metadata about each query executed within our application, so it makes sense for us to store all the metadata in one place.

We're planning on exposing this information back to the client using HTTP response headers. I see that the Gateway code already unpacks the response header from the downstream requests and sends them back to the client; I presume that we'll just have to modify the code to also add the cluster name to the header

@yuokada
Copy link

yuokada commented Sep 11, 2024

That sounds good. But, there are cases that system administrators would like to conceal cluster names, I think.
So, it's better to make it configurable whether we can expose cluster names or not.

@harishnelakurthi
Copy link

harishnelakurthi commented Sep 11, 2024

@yuokada - I have made this change configurable. Can I get a PR review on this please? #466

@yuokada
Copy link

yuokada commented Sep 12, 2024

Thanks. It looks good to me. ✅

@harishnelakurthi
Copy link

@yuokada - can you approve the PR please?

@yuokada
Copy link

yuokada commented Sep 12, 2024

I'm just a user of Trino. Sorry, I have no power to approve your PR.


Anyway, you need to first sign CLA to merge your PR.
#466 (comment)

@mosabua
Copy link
Member

mosabua commented Sep 12, 2024

I'm not convinced that using the HTTP headers and just add the cluster name for now is the right approach. If we add that now we probably end up with more and more requests to add all sorts of other data as HTTPS headers. I'm wondering if it would be better to find an encapsulating entity, maybe a session cookie or something else to just passed back and then individual users can stuff whatever they want into that without us having to adjust code. In fact I think this already might work as desired with the cookie based routing or it would be possible to adjust at least. wdyt @willmostly

@harishnelakurthi
Copy link

@mosabua - Not sure if I followed completely. Can you please point me the code on where the destination cluster info stuffed in the cookie today? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants