fix: treat instanceId metadata as a number#713
Conversation
googlebot
added
the
cla: yes
| const labels: LabelObject = {}; | ||
| this.defaultLabels = {}; | ||
| // tslint:disable-next-line:no-any | ||
| const labels: {[key: string]: any} = {}; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| this.defaultLabels = labels; | ||
| // Coerce values to strings. | ||
| for (const key of Object.keys(labels)) { | ||
| this.defaultLabels[key] = `${labels[key]}`; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #713 +/- ##
==========================================
+ Coverage 90.77% 90.78% +<.01%
==========================================
Files 29 29
Lines 1485 1486 +1
Branches 293 293
==========================================
+ Hits 1348 1349 +1
Misses 57 57
Partials 80 80
Continue to review full report at Codecov.
|
| labels[TraceLabels.GCE_HOSTNAME] = hostname; | ||
| if (instanceId) { | ||
| labels[TraceLabels.GCE_INSTANCE_ID] = instanceId; | ||
| labels[TraceLabels.GCE_INSTANCE_ID] = `${instanceId}`; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| this.defaultLabels = {}; | ||
| addDefaultLabel( | ||
| TraceLabels.AGENT_DATA, | ||
| 'node ' + pjson.name + ' v' + pjson.version); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
3 participants
Fixes #712
This PR accounts for a change in #680 in how trace labels are applied. By directly assigning fields in a spans' labels instead of using
addLabel, we were assuming that all default labels were strings. This isn't the case, because GCP instance IDs are numbers.