Skip to content

fix: Fix OpAMP url, expose ports & chart maintenance#19

Merged
teeohhem merged 5 commits intomainfrom
tom/fix-opamp-url
May 29, 2025
Merged

fix: Fix OpAMP url, expose ports & chart maintenance#19
teeohhem merged 5 commits intomainfrom
tom/fix-opamp-url

Conversation

@teeohhem
Copy link
Contributor

@teeohhem teeohhem commented May 29, 2025

  • Adds a protocol to the opamp default value
  • Exposes opamp port correctly in app service & deployment specs
  • Adds env var to collector config to specify default clickhouse database
  • Fixes error with storageclass names, preventing chart upgrades
  • Adds lots of tests
  • Update readme for instructions for self-app instrumentation after install.

Ref: HDX-1805

- ReadWriteOnce
{{- if .Values.global.storageClass }}
storageClassName: {{ .Values.global.storageClass }}
{{- end }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this prevented chart upgrades

- port: {{ .Values.hyperdx.opampPort }}
targetPort: {{ .Values.hyperdx.opampPort }}
name: opamp
selector:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

expose opamp port in the service

{{- end }}
- name: OPAMP_SERVER_URL
value: {{ .Values.otel.opampServerUrl | default (printf "%s-app:%v" (include "hdx-oss.fullname" .) .Values.hyperdx.opampPort ) }}
value: {{ .Values.otel.opampServerUrl | default (printf "http://%s-app:%v" (include "hdx-oss.fullname" .) .Values.hyperdx.opampPort ) }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

value was missing http protocol in default value

imageRegistry: ""
imagePullSecrets: []
storageClassName: []
storageClassName: "local-path"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bad default value...should be a string. industry standard is "local-path" as a default

@teeohhem teeohhem changed the title fix: add protocol to default opamp url fix: Fix OpAMP url, expose ports & chart maintenance May 29, 2025
@teeohhem teeohhem merged commit 8f4df6c into main May 29, 2025
1 check passed
@teeohhem teeohhem deleted the tom/fix-opamp-url branch May 29, 2025 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants