Update http Response code attribute names #513
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
meiao
changed the title
meiao
force-pushed
the
aonuki/http-status-code
branch
from
854f3ce
to
fc659c0
Compare
Creates a new constructor that receives the Builder, so no changes on the constructor are needed when Builder is updated
meiao
force-pushed
the
aonuki/http-status-code
branch
from
fc659c0
to
8ca9fa4
Compare
twcrone
reviewed
Nov 5, 2021
instrumentation-test/src/main/java/com/newrelic/agent/introspec/ExternalRequest.java
Show resolved
Hide resolved
twcrone
reviewed
Nov 5, 2021
...umentation-test/src/main/java/com/newrelic/agent/introspec/internal/ExternalRequestImpl.java
Outdated
Show resolved
Hide resolved
twcrone
reviewed
Nov 5, 2021
| } | ||
|
|
||
| @Test | ||
| public void testGetBucketLocation() { | ||
| createBucketNoTxn(); | ||
| getBucketLocation(); | ||
| assertMetrics("getBucketLocation"); | ||
| assertMetrics("getBucketLocation", 200); | ||
| } | ||
|
|
||
| // This test passes but it consistently takes 60s to complete |
There was a problem hiding this comment.
So do we ever uncomment and run this test or should it go?
There was a problem hiding this comment.
We can uncomment it now that we have GE.
twcrone
reviewed
Nov 5, 2021
...-4/src/main/java/org/apache/http/nio/protocol/HttpAsyncResponseConsumer_Instrumentation.java
Outdated
Show resolved
Hide resolved
twcrone
reviewed
Nov 5, 2021
...tp-client-1.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java
Outdated
Show resolved
Hide resolved
twcrone
reviewed
Nov 5, 2021
| @@ -151,6 +153,8 @@ public void testCat() throws Exception { | |||
| ExternalRequest externalRequest = externalRequests.iterator().next(); | |||
| assertEquals(1, externalRequest.getCount()); | |||
| assertEquals(host, externalRequest.getHostname()); | |||
| assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); | |||
| assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it | |||
There was a problem hiding this comment.
These are good examples of comments on the "Why".
twcrone
reviewed
Nov 5, 2021
newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java
Outdated
Show resolved
Hide resolved
twcrone
approved these changes
Nov 5, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This adds the
http.statusCodeandhttp.statusTextto spans and transactions.Related Github Issue
#225
Checks
[y] Are your contributions backwards compatible with relevant frameworks and APIs?
[n] Does your code contain any breaking changes? Please describe.
[n] Does your code introduce any new dependencies? Please describe.