-
Notifications
You must be signed in to change notification settings - Fork 452
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
add http.status_code attribute to all Spans that have a lowLevelHttpResponse #986
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
…a low level http response Signed-off-by: CI-Bot for Emmanuel Courreges <[email protected]>
3c6c674
to
9da74ab
Compare
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.
This library uses OpenCensus, not OpenTelemetry.
Do you happen to know what the status of those two is? We really need to figure that out.
I know but OpenTelemetry needs to be adopted eventually because OpenCensus will disappear in 2 years. On top of this, OpenCensus already uses http.status_code as attribute name, which you can see from the fact that I am using an existing string variable for it.
As you can see here, OpenTelemetry is still a work in progress, so if I were you, I would wait until it is stable and production ready to migrate to it, but it will be fairly straightforward because there is an OpenCensus bridge, and also the interfaces are very similar. |
@@ -1012,6 +1012,7 @@ public HttpResponse execute() throws IOException { | |||
LowLevelHttpResponse lowLevelHttpResponse = lowLevelHttpRequest.execute(); | |||
if (lowLevelHttpResponse != null) { | |||
OpenCensusUtils.recordReceivedMessageEvent(span, lowLevelHttpResponse.getContentLength()); | |||
span.putAttribute(HttpTraceAttributeConstants.HTTP_STATUS_CODE, AttributeValue.longAttributeValue(lowLevelHttpResponse.getStatusCode())); |
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.
Why is this a long instead of an int? Status codes don't go past 5XX
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.
Because there is only long for integer attribute types in OpenCensus: https://javadoc.io/doc/io.opencensus/opencensus-api/0.12.2/io/opencensus/trace/AttributeValue.html
@googlebot I signed it! |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
🤖 I have created a release \*beep\* \*boop\* --- ## [1.39.0](https://github.com/googleapis/google-http-java-client/compare/v1.38.1...v1.39.0) (2021-02-24) ### Features * add http.status_code attribute to all Spans that have at least a low level http response ([#986](https://github.com/googleapis/google-http-java-client/issues/986)) ([fb02042](https://github.com/googleapis/google-http-java-client/commit/fb02042ac216379820950879cea45d06eec5278c)) ### Bug Fixes * deprecate obsolete utility methods ([#1231](https://github.com/googleapis/google-http-java-client/issues/1231)) ([8f95371](https://github.com/googleapis/google-http-java-client/commit/8f95371cf5681fbc67bd598d74089f38742a1177)) * fix buildRequest setUrl order ([#1255](https://github.com/googleapis/google-http-java-client/issues/1255)) ([97ffee1](https://github.com/googleapis/google-http-java-client/commit/97ffee1a68af6637dd5d53fcd70e2ce02c9c9604)) * refactor to use StandardCharsets ([#1243](https://github.com/googleapis/google-http-java-client/issues/1243)) ([03ec798](https://github.com/googleapis/google-http-java-client/commit/03ec798d7637ff454614415be7b324cd8dc7c77c)) * remove old broken link ([#1275](https://github.com/googleapis/google-http-java-client/issues/1275)) ([12f80e0](https://github.com/googleapis/google-http-java-client/commit/12f80e09e71a41b967db548ab93cab2e3f4e549c)), closes [#1278](https://github.com/googleapis/google-http-java-client/issues/1278) * remove unused logger ([#1228](https://github.com/googleapis/google-http-java-client/issues/1228)) ([779d383](https://github.com/googleapis/google-http-java-client/commit/779d3832ffce741b7c4055a14855ce8755695fce)) ### Documentation * Jackson is unable to maintain stable Javadocs ([#1265](https://github.com/googleapis/google-http-java-client/issues/1265)) ([9e8fcff](https://github.com/googleapis/google-http-java-client/commit/9e8fcfffc6d92505528aff0a89c169bf3e812c41)) ### Dependencies * update dependency com.google.protobuf:protobuf-java to v3.15.1 ([#1270](https://github.com/googleapis/google-http-java-client/issues/1270)) ([213726a](https://github.com/googleapis/google-http-java-client/commit/213726a0b65f35fdc65713027833d22b553bbc20)) * update dependency com.google.protobuf:protobuf-java to v3.15.2 ([#1284](https://github.com/googleapis/google-http-java-client/issues/1284)) ([dfa06bc](https://github.com/googleapis/google-http-java-client/commit/dfa06bca432f644a7146e3987555f19c5d1be7c5)) * update OpenCensus to 0.28.0 for consistency with gRPC ([#1242](https://github.com/googleapis/google-http-java-client/issues/1242)) ([b810d53](https://github.com/googleapis/google-http-java-client/commit/b810d53c8f63380c1b4f398408cfb47c6ab134cc)) * version manage error_prone_annotations to 2.5.1 ([#1268](https://github.com/googleapis/google-http-java-client/issues/1268)) ([6a95f6f](https://github.com/googleapis/google-http-java-client/commit/6a95f6f2494a9dafd968d212b15c9b329416864f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This one is fairly straight forward:
It adds the official OpenTelemetry http.status_code attribute which is mandatory, was missing and is very useful to know what happened to one's request.
Signed-off-by: CI-Bot for Emmanuel Courreges [email protected]