Skip to content
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

Merged
merged 1 commit into from Jan 19, 2021

Conversation

ecourreges-orange
Copy link
Contributor

@ecourreges-orange ecourreges-orange commented Mar 2, 2020

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 emmanuel.courreges@orange.com

@ecourreges-orange ecourreges-orange requested a review from as a code owner Mar 2, 2020
@googlebot
Copy link

@googlebot googlebot commented Mar 2, 2020

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 @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no label Mar 2, 2020
…a low level http response

Signed-off-by: CI-Bot for Emmanuel Courreges <emmanuel.courreges@orange.com>
Copy link
Collaborator

@elharo elharo left a comment

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.

@elharo elharo added the kokoro:run label Mar 2, 2020
@kokoro-team kokoro-team removed the kokoro:run label Mar 2, 2020
@ecourreges-orange
Copy link
Contributor Author

@ecourreges-orange ecourreges-orange commented Mar 3, 2020

This library uses OpenCensus, not OpenTelemetry.

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.

Do you happen to know what the status of those two is? We really need to figure that out.

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()));
Copy link
Collaborator

@elharo elharo Mar 3, 2020

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

Copy link
Contributor Author

@ecourreges-orange ecourreges-orange Mar 3, 2020

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

@ecourreges-orange
Copy link
Contributor Author

@ecourreges-orange ecourreges-orange commented Sep 8, 2020

@googlebot I signed it!

@ecourreges-orange
Copy link
Contributor Author

@ecourreges-orange ecourreges-orange commented Jan 19, 2021

@googlebot I signed it!

@google-cla
Copy link

@google-cla google-cla bot commented Jan 19, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: yes and removed cla: no labels Jan 19, 2021
elharo
elharo approved these changes Jan 19, 2021
@elharo elharo merged commit fb02042 into googleapis:master Jan 19, 2021
9 checks passed
gcf-merge-on-green bot pushed a commit that referenced this issue Feb 24, 2021
🤖 I have created a release \*beep\* \*boop\* 
---
## [1.39.0](https://www.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://www.github.com/googleapis/google-http-java-client/issues/986)) ([fb02042](https://www.github.com/googleapis/google-http-java-client/commit/fb02042ac216379820950879cea45d06eec5278c))


### Bug Fixes

* deprecate obsolete utility methods ([#1231](https://www.github.com/googleapis/google-http-java-client/issues/1231)) ([8f95371](https://www.github.com/googleapis/google-http-java-client/commit/8f95371cf5681fbc67bd598d74089f38742a1177))
* fix buildRequest setUrl order ([#1255](https://www.github.com/googleapis/google-http-java-client/issues/1255)) ([97ffee1](https://www.github.com/googleapis/google-http-java-client/commit/97ffee1a68af6637dd5d53fcd70e2ce02c9c9604))
* refactor to use StandardCharsets ([#1243](https://www.github.com/googleapis/google-http-java-client/issues/1243)) ([03ec798](https://www.github.com/googleapis/google-http-java-client/commit/03ec798d7637ff454614415be7b324cd8dc7c77c))
* remove old broken link ([#1275](https://www.github.com/googleapis/google-http-java-client/issues/1275)) ([12f80e0](https://www.github.com/googleapis/google-http-java-client/commit/12f80e09e71a41b967db548ab93cab2e3f4e549c)), closes [#1278](https://www.github.com/googleapis/google-http-java-client/issues/1278)
* remove unused logger ([#1228](https://www.github.com/googleapis/google-http-java-client/issues/1228)) ([779d383](https://www.github.com/googleapis/google-http-java-client/commit/779d3832ffce741b7c4055a14855ce8755695fce))


### Documentation

* Jackson is unable to maintain stable Javadocs ([#1265](https://www.github.com/googleapis/google-http-java-client/issues/1265)) ([9e8fcff](https://www.github.com/googleapis/google-http-java-client/commit/9e8fcfffc6d92505528aff0a89c169bf3e812c41))


### Dependencies

* update dependency com.google.protobuf:protobuf-java to v3.15.1 ([#1270](https://www.github.com/googleapis/google-http-java-client/issues/1270)) ([213726a](https://www.github.com/googleapis/google-http-java-client/commit/213726a0b65f35fdc65713027833d22b553bbc20))
* update dependency com.google.protobuf:protobuf-java to v3.15.2 ([#1284](https://www.github.com/googleapis/google-http-java-client/issues/1284)) ([dfa06bc](https://www.github.com/googleapis/google-http-java-client/commit/dfa06bca432f644a7146e3987555f19c5d1be7c5))
* update OpenCensus to 0.28.0 for consistency with gRPC ([#1242](https://www.github.com/googleapis/google-http-java-client/issues/1242)) ([b810d53](https://www.github.com/googleapis/google-http-java-client/commit/b810d53c8f63380c1b4f398408cfb47c6ab134cc))
* version manage error_prone_annotations to 2.5.1 ([#1268](https://www.github.com/googleapis/google-http-java-client/issues/1268)) ([6a95f6f](https://www.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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants