fix(grpc): handle None values for tags #2443
Conversation
|
@Mergifyio backport 0.48 |
|
Command
|
Kyle-Verhoog
left a comment
Kyle-Verhoog
left a comment
There was a problem hiding this comment.
Hmm we've been bitten a few times now with _set_str_tag and None. I think the solution here is good for now but perhaps we should handle the case in _set_str_tag since we can't get type verification in our integrations.
| (None, None, [mock.call("span.kind", "client")]), | ||
| ], | ||
| ) | ||
| def test_set_grpc_client_meta(host, port, calls): |
There was a problem hiding this comment.
I'm not a big fan of this pattern because it's a bit removed from what could happen realistically in the integration - but I also understand that it's hard to replicate these scenarios
There was a problem hiding this comment.
Right, I think in this case I opted to go the unit testing route rather than an integration test because it's unclear how we could reproduce such a scenario. We could spend some time moving forward with rethinking these tests though.
* fix(grpc): handle None values for tags * handle empty hostname in Python 2.7 * correct coercion of hostname Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit c08f02a)
|
Command
|
* fix(grpc): handle None values for tags * handle empty hostname in Python 2.7 * correct coercion of hostname Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit c08f02a) Co-authored-by: Tahir H. Butt <tahir.butt@datadoghq.com> Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com> Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
4 participants
Description
Since #1938 we had exposed how we were not checking values before setting tags, causing
TypeErrors to be raised if the value was being set toNone. This PR adds checks around those cases where the value can beNone.Checklist