GoogleCloudPlatform / opentelemetry-operations-java

Apache License 2.0
71 stars 41 forks source link

Fix #340 bug #342

Closed tkmsaaaam closed 3 months ago

tkmsaaaam commented 3 months ago

Fix #340 's bug.

I made bellow changes.

1. Use appropriate CLOUD_PROVIDER .

2. Use appropriate CLOUD_PLATFORM.

3. Add test cases about GCP .

4. Fix to map resource when platform == null

I have to map to k8s resources when this condition. If i did this.

2. Use appropriate CLOUD_PLATFORM.

Then I need duplicate these lines https://github.com/GoogleCloudPlatform/opentelemetry-operations-java/pull/342/files#diff-992354fc4c9a5bcfac13812fb9e420d0498483ae56b986f25e71d6649386de99L187-L198 . So I make mapK8sResourceOrGenericTaskOrNode to refactor it.

tkmsaaaam commented 3 months ago

I update description. https://github.com/GoogleCloudPlatform/opentelemetry-operations-java/pull/342#issue-2290726664

tkmsaaaam commented 3 months ago

Fix https://github.com/GoogleCloudPlatform/opentelemetry-operations-java/pull/340 's bug.

Use appropriate CLOUD_PROVIDER . Add test cases about GCP . Fix to map resource when platform == null Regarding use of appropriate cloud provider, I think GCP is correct, since GCP has Monitored Resource types for other cloud providers like AWS too.

If it is true. I don't need changes about below. I will fix it.

1. Use appropriate CLOUD_PROVIDER .

psx95 commented 3 months ago

Thank you for the clarification, I will review this shortly!

tkmsaaaam commented 3 months ago

Thanks for reporting and fixing this bug! Could you address the last pending comment

Thanks for your review. I've done it.

psx95 commented 3 months ago

/gcbrun

psx95 commented 3 months ago

Fix #340 's bug. Use appropriate CLOUD_PROVIDER . Add test cases about GCP . Fix to map resource when platform == null Regarding use of appropriate cloud provider, I think GCP is correct, since GCP has Monitored Resource types for other cloud providers like AWS too.

If it is true. I don't need changes about below. I will fix it.

1. Use appropriate CLOUD_PROVIDER .

For posterity, the CLOUD_PROVIDER resource attribute is not added to the MR resource, as such its value does not affect the mapped resource. However, the more appropriate CLOUD_PROVIDER values should be specific to the CLOUD_PLATFORM as indicated by the tests in this PR.