Skip to content

Properly set "identity" in query metrics.#5330

Merged
gianm merged 2 commits intoapache:masterfrom
gianm:fix-queryMetrics-identity
Feb 6, 2018
Merged

Properly set "identity" in query metrics.#5330
gianm merged 2 commits intoapache:masterfrom
gianm:fix-queryMetrics-identity

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented Feb 2, 2018

This patch adds an "identity" field to QueryPlus and sets it in
QueryLifecycle when the query starts executing. This is important
because it allows it to be used for future QueryMetrics created
by that QueryPlus object.

We also add "identity" to the request-level QueryMetrics object
created in emitLogsAndMetrics.

This patch adds an "identity" field to QueryPlus and sets it in
QueryLifecycle when the query starts executing. This is important
because it allows it to be used for future QueryMetrics created
by that QueryPlus object.

We also add "identity" to the request-level QueryMetrics object
created in emitLogsAndMetrics.
@gianm gianm added the Security label Feb 2, 2018
@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Feb 2, 2018

Note that with this change, the broker will report the identity of the actual user, but the historicals will report the system identity. This is because once the broker verifies that the query is allowed, it uses its own credentials to talk to historicals, not the credentials of the original user. So the historicals don't know who the original user is.

I think it would be nice if they did (maybe an 'impersonation' feature would allow the broker to pass along the original identity) but I also think that is out of scope for this particular PR.

@jon-wei
Copy link
Copy Markdown
Contributor

jon-wei commented Feb 2, 2018

Looks like there's a real inspection error:

processing/src/main/java/io/druid/query
 QueryPlus.java (1)
69: getIdentity() Method is never used.

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Feb 6, 2018

Removed the unused method and pushed again.

@gianm gianm merged commit c21ff6e into apache:master Feb 6, 2018
@gianm gianm deleted the fix-queryMetrics-identity branch February 6, 2018 18:54
gianm added a commit to implydata/druid-public that referenced this pull request Feb 6, 2018
* Properly set "identity" in query metrics.

This patch adds an "identity" field to QueryPlus and sets it in
QueryLifecycle when the query starts executing. This is important
because it allows it to be used for future QueryMetrics created
by that QueryPlus object.

We also add "identity" to the request-level QueryMetrics object
created in emitLogsAndMetrics.

* Remove unused method.
gianm added a commit to implydata/druid-public that referenced this pull request May 11, 2018
* Properly set "identity" in query metrics.

This patch adds an "identity" field to QueryPlus and sets it in
QueryLifecycle when the query starts executing. This is important
because it allows it to be used for future QueryMetrics created
by that QueryPlus object.

We also add "identity" to the request-level QueryMetrics object
created in emitLogsAndMetrics.

* Remove unused method.
@dclim dclim added this to the 0.13.0 milestone Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants