Skip to content
This repository was archived by the owner on Sep 17, 2025. It is now read-only.

Replace gke_container with k8s_container for stackdriver#528

Merged
c24t merged 13 commits intocensus-instrumentation:masterfrom
c24t:k8s-in-gke-out
Mar 4, 2019
Merged

Replace gke_container with k8s_container for stackdriver#528
c24t merged 13 commits intocensus-instrumentation:masterfrom
c24t:k8s-in-gke-out

Conversation

@c24t
Copy link
Copy Markdown
Member

@c24t c24t commented Feb 27, 2019

This PR replaces the gke_container monitored resource type with the new k8s_container for stackdriver following the migration guide.

It also moves the detection utils for kubernetes out of gcp_metadata_config and into a new module. It still depends on GCP metadata, so it won't work on non-GKE kubernetes environments, but this does match the behavior of the other clients.

See census-instrumentation/opencensus-java#1467 for the same change in the java client.

This PR fixes #510 and unblocks #515.

@songy23 @mayurkale22 there's a good chance I'm still missing something here, please take a look.

from opencensus.common.monitored_resource import k8s_utils


class TestK8SUtils(unittest.TestCase):
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These were moved from test_gcp_metadata_config.py. Note that instance_id label doesn't appear anymore.

_K8S_ATTRIBUTES = {
# ProjectID is the identifier of the GCP project associated with this
# resource, such as "my-project".
'project_id': 'project/project-id',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

project-id and location aren't part of the K8S resource. For K8S container resource we have:

  • k8s.io/cluster/name
  • k8s.io/namespace/name
  • k8s.io/pod/name
  • k8s.io/container/name

See the latest Java implementation: https://github.com/census-instrumentation/opencensus-java/blob/master/contrib/resource_util/src/main/java/io/opencensus/contrib/resource/util/K8sContainerResource.java.

I'll update the specs in a minute.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, updated in baa06f1.

1. 'gke_container'
- https://cloud.google.com/monitoring/api/resources#tag_gke_container
1. 'k8s_container'
- https://cloud.google.com/monitoring/api/resources#tag_k8s_container
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Those URLs (including the ones below) shouldn't point to Stackdriver documentation, since this is our core library (not exporters). Please update this to point to the specs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I dropped the whole comment since we link to the resource spec from the Resource docstring.

Copy link
Copy Markdown
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 except one minor nit.

POD_NAME_KEY = 'k8s.io/pod/name'

# Container name
CONTAINER_NAME_KEY = 'k8s.io/container'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@c24t c24t merged commit 1d8bf1c into census-instrumentation:master Mar 4, 2019
@c24t c24t deleted the k8s-in-gke-out branch March 4, 2019 19:53
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.

Replace gke_container resource with k8s_container

3 participants