search

GoogleCloudPlatform / opentelemetry-demo

Apache License 2.0
14 stars 7 forks source link

Support `kubectl apply` steps #37

Closed damemi closed 5 months ago

damemi commented 5 months ago

Fixes https://github.com/GoogleCloudPlatform/opentelemetry-demo/issues/24

Update kubernetes/opentelemetry-demo.yaml file to match our Helm and docker-compose deployments. This file is generated in the upstream demo repo, but since we aren't generating it we can edit it directly and manually handle conflicts for now.

This also makes some updates to the Workload Identity steps to align the Helm/Kubectl install. By default, kubectl apply will install the collector in the otel-demo namespace while Helm installs in default. Instead of making more changes to the kubectl manifests or having 2 sets of instructions I just updated the commands to Helm install in the otel-demo namespace too.

In this change, I noticed that some of the processors that were removed in the Helm chart (see https://github.com/GoogleCloudPlatform/opentelemetry-demo/issues/23) were still present in this file. I don't know if that's because of catching this in the middle of an upstream release, or if upstream just didn't update that file, but I manually tested this and it still deploys. When we have integration testing that will have to check this when we rebase for the next release.

Tested both kubectl apply and Helm install manually.

dashpole commented 5 months ago

Will this make rebases on upstream really hard? IIRC, the plan was to try and automate those to some extent.

damemi commented 5 months ago

Will this make rebases on upstream really hard? IIRC, the plan was to try and automate those to some extent.

I don't think so, this will be one file that we'll have to resolve minimal conflicts with. Automating that will be tough, so maybe we could look at a different way to do this (like an entirely separate config file that gets kubectl apply'd to overwrite the default one?)