Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2792 +/- ##
============================================
+ Coverage 69.95% 70.66% +0.70%
- Complexity 10534 10622 +88
============================================
Files 872 881 +9
Lines 43009 42913 -96
Branches 6589 6486 -103
============================================
+ Hits 30087 30324 +237
+ Misses 10012 9670 -342
- Partials 2910 2919 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
...ore-4.0.0/src/main/java/com/nr/agent/instrumentation/micronaut/NRBiConsumerTokenWrapper.java
Outdated
Show resolved
Hide resolved
|
|
||
| StringBuilder sb = new StringBuilder(); | ||
| sb.append(methodName); | ||
| sb.append(" - "); |
There was a problem hiding this comment.
This change ended up breaking a few AITs (which was easy to reconcile), is there a precedent for this formatting modification?
There was a problem hiding this comment.
yeah, it turned out that there was an inconsistency in transaction naming. depending on the flow some transactions would get method name at the end and some at the beginning. This one was changed to be the same as the other transaction. We can go either way, I just think it should be consistent.
There was a problem hiding this comment.
Agreed. If this is following a standard transaction convention then this change should be fine.
| sb.append(" (").append(methodName).append(") "); | ||
| NewRelic.getAgent().getTransaction().setTransactionName(TransactionNamePriority.FRAMEWORK_HIGH, true, "MicronautController", sb.toString()); | ||
| String txnName = sb.toString().replace("//", "/"); | ||
| NewRelic.getAgent().getTransaction().setTransactionName(TransactionNamePriority.FRAMEWORK_LOW , false, "MicronautController", txnName); |
There was a problem hiding this comment.
Why has the transaction name priority been downgraded?
There was a problem hiding this comment.
this code was overriding a previously set that was better suited name to describe the transaction. At a minimum the override flag needed to be changed to get rid of the override. Perhaps the downgrade isn't truly needed. Probably changed it to ensure it didn't override
There was a problem hiding this comment.
Understood. Was just curious about the rationale. I don't believe the downgrade is a deal breaker, and I would defer to your judgment here.
|
I noticed two new HTTP client modules were added ( |
|
It's worth noting that building/running a agent JAR off of this branch results in passing Micronaut draft AITs, whereas they were failing prior. |
|
@sharvath-newrelic |
|
moved new Http Client modules to PR: #2834 |
3 participants
Fixes problems related to https://new-relic.atlassian.net/browse/NR-523865
Fixed problems related to excessive number of unexpired async tokens.