Filter Temporal exit errors from traces #18777
Conversation
jdpgrailsdev
requested review from
colesnodgrass,
gosusnp,
benmoriceau and
alovew
github-actions
bot
added
area/platform
|
|
||
| class DummySpan implements MutableSpan { | ||
|
|
||
| @SuppressWarnings("PMD") |
There was a problem hiding this comment.
Nit: could we be more specific here. I wonder what is the error here since ti looks that the variable is being used.
There was a problem hiding this comment.
@colesnodgrass Any insight as to why this suppression was added? @benmoriceau This was a copy/extraction from the existing test, so I don't have the context.
There was a problem hiding this comment.
I meant to clean this up before committing it. Since there was a 30 minute turn around time between making a change and getting it into dev to verify it, I didn't want the build failing due to a PMD issue (making that 30 minutes even longer). Once things were working as expected I should have fixed this to suppress only the specific issue (if any even exist anymore).
There was a problem hiding this comment.
I did some testing locally and this suppression does not appear to be necessary. I've removed it and will verify that the PR builds without issue.
There was a problem hiding this comment.
Looks like it could be removed safely.
|
|
||
| @Override | ||
| public Collection<? extends MutableSpan> onTraceComplete(final Collection<? extends MutableSpan> trace) { | ||
| final var filtered = new ArrayList<MutableSpan>(); |
There was a problem hiding this comment.
Nit; we could use a kotlin/scala like syntax here by using val. It is not standard so feel free to ignore it.
* Filter Temporal exit errors from traces * Remove unnecessary PMD suppression * Formatting
What
How
Recommended reading order
TemporalSdkInterceptor.javaTests