Skip to content
This repository was archived by the owner on Oct 3, 2023. It is now read-only.

Refactor resource util to match specs.#485

Merged
mayurkale22 merged 2 commits intocensus-instrumentation:masterfrom
mayurkale22:newresource
May 28, 2019
Merged

Refactor resource util to match specs.#485
mayurkale22 merged 2 commits intocensus-instrumentation:masterfrom
mayurkale22:newresource

Conversation

@mayurkale22
Copy link
Member

@mayurkale22 mayurkale22 commented Apr 12, 2019

@codecov-io
Copy link

codecov-io commented Apr 12, 2019

Codecov Report

Merging #485 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #485      +/-   ##
==========================================
+ Coverage   95.25%   95.27%   +0.02%     
==========================================
  Files         147      148       +1     
  Lines       10470    10494      +24     
  Branches      739      734       -5     
==========================================
+ Hits         9973     9998      +25     
+ Misses        497      496       -1
Impacted Files Coverage Δ
test/test-tracecontext-format.ts 90.9% <0%> (-4.44%) ⬇️
src/stackdriver-monitoring.ts 77.02% <0%> (-2.71%) ⬇️
src/tracecontext-format.ts 91.66% <0%> (-2.57%) ⬇️
test/test-detect-resource.ts 99.01% <0%> (-0.01%) ⬇️
src/resource-labels.ts 100% <0%> (ø) ⬆️
...c/zpages-frontend/controllers/tracez.controller.ts 100% <0%> (ø) ⬆️
...es-frontend/controllers/traceconfigz.controller.ts 100% <0%> (ø) ⬆️
test/test-stackdriver-monitoring-utils.ts 100% <0%> (ø) ⬆️
src/zpages-frontend/controllers/rpcz.controller.ts 100% <0%> (ø) ⬆️
test/test-prometheus-stats.ts 100% <0%> (ø) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca0e962...69f6f6e. Read the comment docs.

Object.keys(mappings).forEach((key) => {
if (autoDetectedResource.labels[mappings[key]]) {
if (mappings[key] === resource.AWS_REGION_KEY) {
if (mappings[key] === CLOUD_RESOURCE.REGION_KEY) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also need to check if the CloudProvider is AWS.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, checking type is AWS_EC2_INSTANCE and key is REGION_KEY before adding aws: prefix. Sorry for the late response.

@mayurkale22 mayurkale22 force-pushed the newresource branch 2 times, most recently from cb9055b to 2a99262 Compare May 17, 2019 18:02
Copy link
Contributor

@songy23 songy23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mayurkale22 mayurkale22 merged commit a157106 into census-instrumentation:master May 28, 2019
@mayurkale22 mayurkale22 deleted the newresource branch May 28, 2019 20:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor resource util to match specs

5 participants