starburstdata / metabase-driver

Starburst Metabase driver
Apache License 2.0
60 stars 11 forks source link

It might be possible to propagate UserId in the EXECUTE #55

Open digarcia78 opened 1 year ago

digarcia78 commented 1 year ago

Driver version :- 1.0.6

I have a dashboard with filter using that driver. Metabase sends two commands to Trino:

1- PREPARE statement1 FROM -- Metabase:: userID: .... [SQL] 2- EXECUTE statement1 USING [ARGS]

In the first command (prepare) there is a parameter that allows to identify the user (userID)

Example: -- Metabase:: userID: 73 queryType: native queryHash: c21cbb35367f5676d7832f7565296b9e91e2dbe0bf16e9bbde7893843d66f383

It allows to debug, audit and make stats per users.

The execute command, the userID\queryHash is lost.

It might be possible to propagate that information in the EXECUTE call too?

Another alternative could be a setting to disable prepared statements.

leniartek commented 1 year ago

Hi @digarcia78 thanks for your suggestion, are you using Starburst or Trino?

Can you elaborate what is the benefit of adding UserID to the execute command? Is it not possible for you to identify the User by the same QueryID in both PREPARE and EXECUTE commands?

digarcia78 commented 1 year ago

Hi @leniartek, thank you for the answer.

We observe that the queryId is different in both PREPARE and EXECUTE commands. How can you relate the queryId?

We have multiple instances of Metabase in combination with a load balancer to direct traffic to the instances.

The identification of the user in both commands allows us to have audit information and identify the executions of each user. It also allows us to assign the group resource, and in the Trino UI to know who is executing each query

We use the deprecated presto driver which does not generate both commands. In this way, it simplifies the audit and reduce the number of requests to Trino.

andrewdibiasio6 commented 1 year ago

The difference is the deprecated presto driver does not use prepare statements at all. We will have to find a way to pass the comment to the execution of the prepare statements.

leniartek commented 1 year ago

@andrewdibiasio6 I thought it's not possible to get rid of prepare statements due to the Metabase issue with parsing inline comments?

leniartek commented 1 year ago

@digarcia78 thanks for additional details. Do you use OSS Trino or Starburst (Enterprise/Galaxy)?

We observe that the queryId is different in both PREPARE and EXECUTE commands. How can you relate the queryId? Your are correct, I was wrong, unfortunately there is no way to relate these two commands, they will have separate queryId because they are indeed two separate queries, even if triggered from one Metabase call.

The identification of the user in both commands allows us to have audit information and identify the executions of each user. It also allows us to assign the group resource, and in the Trino UI to know who is executing each query In Trino UI you can see the User who triggered query, my assumption is that you use Service Account for Metabase authentication (always same user) so thats why you need a UserID in the comment field?

Screenshot 2022-11-30 at 14 47 48
digarcia78 commented 1 year ago

Hi @leniartek

asronihidayat commented 9 months ago

Any update for this?

In my case, i need both (userID and queryHash in EXECUTE command).

Thanks!

leniartek commented 9 months ago

@asronihidayat @digarcia78 we are working on a change that will remove the need of using Prepared Statements, as a result you will have just 1 query instead of two (PREPARE, EXECUTE), all the parameters will be available for this single query.

digarcia78 commented 6 months ago

Hello @leniartek, any news about this issue?

leniartek commented 6 months ago

Hello @leniartek, any news about this issue?

Yes @digarcia78, since version 4.1.0 you can use Optimized prepared statement option which should solve your problem. Please let me know if this works.

digarcia78 commented 3 months ago

Hello @leniartek, I have Trino v406. Is it a requirement to upgrade to trino v431? How should I use the Optimized prepared statement option ?