Extended SetAttribute() method to include all integer types#6
Conversation
| { | ||
| (void)name; | ||
| (void)timestamp; | ||
| (void)attributes; |
There was a problem hiding this comment.
Should I go with unnamed parameters here and below?
Not sure what the specification is under the GCP org for unused parameters.
There was a problem hiding this comment.
I am not sure what is the spec, but I think those parameters are unused for a short term.
Eventually this code AddEvent should eventually map to a TimeEvent (https://cloud.google.com/trace/docs/reference/v2/rpc/google.devtools.cloudtrace.v2#timeevent).
IlyaKobelevskiy
left a comment
IlyaKobelevskiy
left a comment
There was a problem hiding this comment.
Looks good overall, just one minor nit.
| const common::AttributeValue &value) noexcept | ||
| { | ||
| // Get the protobuf span's map | ||
| auto map = span_.mutable_attributes()->mutable_attribute_map(); |
There was a problem hiding this comment.
auto -> auto* for better readability
| { | ||
| (void)name; | ||
| (void)timestamp; | ||
| (void)attributes; |
There was a problem hiding this comment.
I am not sure what is the spec, but I think those parameters are unused for a short term.
Eventually this code AddEvent should eventually map to a TimeEvent (https://cloud.google.com/trace/docs/reference/v2/rpc/google.devtools.cloudtrace.v2#timeevent).
3 participants
Added code to cover all integer types for the GCP recordable.
Added test coverage for the same.