Hotfix/django flask pyramid status code#755
Conversation
|
I haven't taken a deep look into the system test failures. It seems some of the system tests are looking for span labels instead of attributes, and label value is always string. |
|
I can update the tests to get the attributes instead, if possible! |
|
I haven't done the archaeology, would be curious to understand why we had labels instead of attributes in the first place. |
|
Who might know this? And is this a block for merging? If so, I can either swap for attributes or try to understand why we are using labels! |
|
I don't think it's blocking unless @c24t says otherwise. |
|
Waiting for his input so, and if is a block, I would please ask if @c24t can give an explanation of why we are using labels and if there is any workaround! |
|
@c24t any comments here? |
|
@victoraugustolls sorry for my delayed response. We can move forward by changing the test cases to use attributes instead of labels. Once the CI passed, we're good to merge. |
|
Will update later today! |
songy23
left a comment
There was a problem hiding this comment.
LGTM. Stackdriver should use int for status code too.
|
@songy23 and what about the failing system tests? |
|
It seems that Stackdriver labels are always strings. @victoraugustolls would the system test pass if you convert status from int to str in the Stackdriver exporter? |
|
Google client only accepts labels as strings due to the proto format, and during the conversion inside the client, it loses the status code. If I transform the status code to string, it would work, but wouldn’t be “right”. But I think this could be tracked in another issue so we can move this forward! |
Updating
http.status_codeto be an int, as discussed with @reyang. This affects the following contributions: