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.

Sign up for GitHub

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

Jump to bottom

Where O where did my Error Code go #2340

Closed
ivakegg opened this issue Apr 9, 2024 · 2 comments · Fixed by #2399
Closed

Where O where did my Error Code go #2340

ivakegg opened this issue Apr 9, 2024 · 2 comments · Fixed by #2399
Assignees
@lbschanno

Comments

@ivakegg
Copy link
Collaborator

ivakegg commented Apr 9, 2024

We are no longer placing error codes in our metrics when using the federated query logic (CompositeQueryLogic). E.g. if a query "times out" then the error code should be 500-27.
@lbschanno lbschanno self-assigned this Apr 9, 2024
lbschanno added a commit that referenced this issue Jun 4, 2024
@lbschanno
Ensure CompositeQueryLogic error codes are sent to metrics
0138120 
The query metrics API expects QueryExceptions to be found in the cause
of the original exception passed to BaseQueryMetric.setError(Throwable).
In the case of CompositeQueryLogic, due to the additional wrapping of
exceptions passed from its defined internal logics in a
CompositeLogicException, query exceptions were not in the expected place
in the stack hierarchy.

Update the CompositeLogicException to identify any QueryExceptions
present in the stacks of exception arguments, and ensure that they are
either already a QueryException, or that if they contain a
QueryException in the stack, that they are subsequently wrapped in a
CompositeRaisedQueryException with a copy of the desired error code.

Fixes #2340
@lbschanno lbschanno mentioned this issue Jun 4, 2024
Merged
ivakegg pushed a commit that referenced this issue Jul 3, 2024
@lbschanno
fixes #2340: Ensure CompositeQueryLogic error codes are sent to metri…
00cce87 
…cs (#2399)

The query metrics API expects QueryExceptions to be found in the cause
of the original exception passed to BaseQueryMetric.setError(Throwable).
In the case of CompositeQueryLogic, due to the additional wrapping of
exceptions passed from its defined internal logics in a
CompositeLogicException, query exceptions were not in the expected place
in the stack hierarchy.

Updateid the CompositeLogicException to identify any QueryExceptions
present in the stacks of exception arguments, and ensure that they are
either already a QueryException, or that if they contain a
QueryException in the stack, that they are subsequently wrapped in a
CompositeRaisedQueryException with a copy of the desired error code.
@lbschanno lbschanno reopened this Jul 11, 2024
@lbschanno
Copy link
Collaborator

Reopening issue. Error codes are still missing from metrics. Details from a conversation with Ivan:

Unfortunately I think we need to go back to the drawing board on the "where or where did my error codes go"
(https://salmonrunworkspace.slack.com/archives/DL3L3P9SS/p1720728510515859)
I was testing it out and managed to get some more info on what was actually getting missed
They are on the create call, not necessarily the next call.
So if you have a composite query logic, and the underlying query logic fails because of a syntax error, I think they expect a 500-7. The response supposedly contains that error code, but the query metrics do not reflect that. (edited)
(https://salmonrunworkspace.slack.com/archives/DL3L3P9SS/p1720728663046249)
We should be able to reproduce this fairly easily.
Another error would be 400-16 when "The query contained fields which do not exist in the data dictionary". That should be another one easy to reproduce

@lbschanno
Copy link
Collaborator

Closing ticket. The bug appeared on 7.0.7, and the repo at that tag did not have the bugfix commit in it. I confirmed that applying the bug fix did fix the issue with error codes not appearing metrics. Created tickets #2485 and #2486 to address areas of improvements that can be made for recording error codes to metrics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lbschanno lbschanno

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensure CompositeQueryLogic error codes are sent to metrics NationalSecurityAgency/datawave
2 participants
@ivakegg @lbschanno