-
Notifications
You must be signed in to change notification settings - Fork 4
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
Pva/evsrestapi 538 add error tracking audits #325
Pva/evsrestapi 538 add error tracking audits #325
Conversation
if (e instanceof ResponseStatusException) { | ||
throw e; | ||
} | ||
|
||
logger.error("Unexpected error", e); | ||
String functionName = | ||
StackWalker.getInstance().walk(s -> s.skip(1).findFirst()).get().getMethodName(); | ||
final Terminology term = termUtils.getIndexedTerminology(terminology, elasticQueryService); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would back off of this. Just using the terminology string is probably good here. If we've had an error, then trying to do these lookups might themselves fail. All we really want to do is get the best info we have into the index as soon as possible. So don't inject termUtils or the elasticQueryService. Like let's start simple and work up to this if we need it at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, some comments. they're repetitive so find the pattern and just look for those things and fix them.
src/main/java/gov/nih/nci/evs/api/controller/BaseController.java
Outdated
Show resolved
Hide resolved
src/main/java/gov/nih/nci/evs/api/service/AbstractStardogLoadServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/gov/nih/nci/evs/api/service/AbstractStardogLoadServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/gov/nih/nci/evs/api/service/BaseLoaderService.java
Outdated
Show resolved
Hide resolved
src/main/java/gov/nih/nci/evs/api/service/LoaderServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/gov/nih/nci/evs/api/service/SparqlQueryManagerServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/gov/nih/nci/evs/api/service/SparqlQueryManagerServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/gov/nih/nci/evs/api/service/SparqlQueryManagerServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/gov/nih/nci/evs/api/service/SparqlQueryManagerServiceImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a few minor changes just around a few things I found but nothing significant. I think this is ready to merge.
EVSRESTAPI-538: Add error tracking audits
https://tracker.nci.nih.gov/browse/EVSRESTAPI-538
draft for now to more easily review code changes