add opentelemetry-demo chart

[wph95]

• document
• test

open-telemetry/opentelemetry-demo#153

[dmitryax]

Hi @wph95, thanks for the contribution. Couple of initial suggestions/questions:

• I believe the chart should be called the same name as the repo
• Looks like prometheus is missing. Is this intentional?

[wph95]

• I believe the chart should be called the same name as the repo

done

• Looks like prometheus is missing. Is this intentional?

Yes, the current version does not include metrics (grafana and prometheus)
I'm a little concerned that PR is too big to review, so metric is not included yet. plan add it in next PR :)

[dmitryax]

Yes, the current version does not include metrics (grafana and prometheus)
I'm a little concerned that PR is too big to review, so metric is not included yet. plan add it in next PR :)

Sounds good 👍

[dineshg13]

otel-col is failing for me with error

Error: failed to get config: cannot unmarshal the configuration: error reading top level configuration sections: 1 error(s) decoding:

* '' has invalid keys: config
2022/07/20 17:43:16 collector server run finished with error: failed to get config: cannot unmarshal the configuration: error reading top level configuration sections: 1 error(s) decoding:

* '' has invalid keys: config

This might becoz of bad indentation in exporter section of configmap. I think it would be good idea to yamllint these files.

[wph95]

otel-col is failing for me with error

@dineshg13 fixed it :)

[saurabhdes]

What's the best way to test this PR. I tried below steps after looking at README but ran into below error :
helm repo add open-telemetry https://open-telemetry.github.io/opentelemetry-helm-charts

helm install my-otel-demo open-telemetry/opentelemetry-demo-webstore
Error: INSTALLATION FAILED: failed to download "open-telemetry/opentelemetry-demo-webstore"

The error seems to be happening because the chart for "opentelemetry-demo-webstore" doesn't yet exist in the repo. Any suggestions on how to get workaround this issue so i can test this PR?

helm search repo open-telemetry

NAME CHART VERSION APP VERSION DESCRIPTION
open-telemetry/opentelemetry-collector 0.22.2 0.55.0 OpenTelemetry Collector Helm chart for Kubernetes
open-telemetry/opentelemetry-operator 0.9.4 0.54.0 OpenTelemetry Operator Helm chart for Kubernetes

[TylerHelmuth]

What's the best way to test this PR.

If you do gh pr checkout 280 you'll be able to checkout this PR. Then you'd be able to do normal helm commands by pointing to ./charts/opentelemetry-demo-webstore. For example, you could do helm template testing ./charts/opentelemetry-demo-webstore --output-dir ./testing to see the files that would be rendered during a default install of the chart.

[saurabhdes]

What's the best way to test this PR.

If you do gh pr checkout 280 you'll be able to checkout this PR. Then you'd be able to do normal helm commands by pointing to ./charts/opentelemetry-demo-webstore. For example, you could do helm template testing ./charts/opentelemetry-demo-webstore --output-dir ./testing to see the files that would be rendered during a default install of the chart.

thanks @TylerHelmuth ! I was able to deploy the services successfully in my AKS cluster as well as local Mac on kind cluster.

[dineshg13]

@wph95 what the reason for not including prometheus & grafana ? we do have some metrics & I believe we are working to adding more

[saurabhdes]

I noticed that in Jaeger "cart-service" is missing in this K8s helm setup and present in docker-compose setup - attaching Jaeger pics for both. I do see the cart-service pod running in both cases with no errors, so likely some instrumentation setup in K8s helm case but not sure.

[wph95]

@wph95 what the reason for not including prometheus & grafana ? we do have some metrics & I believe we are working to adding more

#280 (comment)
wph95#1 (wait this pr merge

[TylerHelmuth]

@Allex1 would appreciate your review on this PR.

[JaredTan95]

@wph95 test meanings examples like https://github.com/open-telemetry/opentelemetry-helm-charts/tree/main/charts/opentelemetry-collector/examples?
Will it i added in this PR or another separated?

[wph95]

@wph95 test meanings examples like https://github.com/open-telemetry/opentelemetry-helm-charts/tree/main/charts/opentelemetry-collector/examples? Will it i added in this PR or another separated?

I think it is better to PR separately

[JaredTan95]

@wph95 test meanings examples like https://github.com/open-telemetry/opentelemetry-helm-charts/tree/main/charts/opentelemetry-collector/examples? Will it i added in this PR or another separated?

I think it is better to PR separately

+1

[JaredTan95]

LGTM

[Allex1]

@wph95 Some docs or comments in values would be appropriate for new users. We can probably do this in a future docs pr. Great contribution! Thanks!

[TylerHelmuth]

@wph95 can you add a values.json.schema like https://github.com/open-telemetry/opentelemetry-helm-charts/blob/main/charts/opentelemetry-collector/values.schema.json

[wph95]

@wph95 can you add a values.json.schema like https://github.com/open-telemetry/opentelemetry-helm-charts/blob/main/charts/opentelemetry-collector/values.schema.json

Good suggestion, just added

[dmitryax]

the chart is still located at /charts/opentelemetry-demo-webstore. Can you please update the directory name?

[dmitryax]

Nice :)

[TylerHelmuth]

It's beautiful!

[dmitryax]

LGTM. Thanks @wph95!

[TylerHelmuth]

Couple more questions as it had been a while since I look through things in depth.

[TylerHelmuth]

Suggested change

	{{- end }}
	{{- end }}

[TylerHelmuth]

Thanks @wph95 and everyone else involved! Excited to see this chart help our end users.