fix: support for Cloud Run monitored resource #78
Conversation
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #78 +/- ##
============================================
- Coverage 76.24% 75.96% -0.29%
Complexity 571 571
============================================
Files 42 42
Lines 3334 3353 +19
Branches 232 237 +5
============================================
+ Hits 2542 2547 +5
- Misses 647 660 +13
- Partials 145 146 +1
Continue to review full report at Codecov.
|
| @@ -40,10 +40,13 @@ | |||
| ContainerName("container_name"), | |||
| InstanceId("instance_id"), | |||
| InstanceName("instance_name"), | |||
| Location("location"), | |||
There was a problem hiding this comment.
Is there a reason to use "location" instead of "region"?
There was a problem hiding this comment.
I looked at log entries that get ingested from stdout, and they all have the location attribute. So, I tried to match that.
| @@ -171,6 +185,11 @@ private static String getValue(Label label) { | |||
|
|
|||
| /* Detect monitored Resource type using environment variables, else return global as default. */ | |||
| private static Resource getAutoDetectedResourceType() { | |||
| if (System.getenv("K_SERVICE") != null && | |||
There was a problem hiding this comment.
Steren's note: Note that checking the presence of K_SERVICE and other env vars is not sufficient to differentiate between Cloud Run and any other Knative installation (e.g. Cloud Run for Anthos).
Would adding a check for "KUBERNETES_SERVICE_HOST" == null, fix this issue?
There was a problem hiding this comment.
Yes, that makes sense.
meltsufin
changed the title
|
@chingor13 can you approve? |
Fixes: #71.