-
Notifications
You must be signed in to change notification settings - Fork 83
Otel collector integration #519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Mateusz seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds OpenTelemetry Collector integration to the Netdata Helm chart, enabling the collection of logs from Kubernetes pods and forwarding them to a netdata instance via OTLP protocol.
Changes:
- Added
netdataOpentelemetrydeployment and service for receiving OTLP data - Integrated
opentelemetry-collectoras a subchart dependency for collecting Kubernetes logs - Fixed YAML indentation issues in ingress hosts and child tolerations configurations
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/netdata/values.yaml | Added netdataOpentelemetry configuration section and otel-collector subchart settings; fixed indentation issues |
| charts/netdata/templates/netdata-otel/service.yaml | Created service to expose OTLP endpoint on port 4317 |
| charts/netdata/templates/netdata-otel/secrets.yaml | Created secrets template for OTEL configuration |
| charts/netdata/templates/netdata-otel/persistentvolumeclaim.yaml | Created PVC for OTEL log storage |
| charts/netdata/templates/netdata-otel/deployment.yaml | Created deployment for netdata instance with OTEL plugin enabled |
| charts/netdata/templates/netdata-otel/configmap.yaml | Created configmap template for OTEL configuration |
| charts/netdata/templates/_helpers.tpl | Added helper templates for netdataOpentelemetry configs management |
| charts/netdata/Chart.yaml | Added opentelemetry-collector subchart dependency |
| charts/netdata/Chart.lock | Generated lock file for subchart dependency |
| charts/netdata/charts/opentelemetry-collector-0.115.0.tgz | Added OpenTelemetry Collector subchart package |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@cubic-dev-ai review this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
witalisoft
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure that otel is also enabled when we are running checks
c268d60 to
f065b9d
Compare
f065b9d to
ac92e27
Compare
5c3757b to
4b1dbb2
Compare
|
|
||
| The following table lists the configurable parameters of the netdata chart and their default values. | ||
|
|
||
| {{ template "chart.valuesTableHtml" . }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTML because it renders better the multiline comments and values + adds some extra feats like copy, colors. Any markdown parser will render it no problem anyway.
| - name: Run chart-testing (install) | ||
| run: ct install --target-branch ${{ github.event.repository.default_branch }} | ||
|
|
||
| - name: Run chart-testing (install with OpenTelemetry) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an extra for easier problem catching, in theory we can just use this one and delete the regular install. Good thing is that at the end of the ct install there is a cleanup so they are both independent.
|
Last thing left for now is to make docs a bit more readable, after that it should be ready for review |
c4cc83f to
73258ee
Compare
73258ee to
138f84e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 14 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a15734d to
3a77eb2
Compare
install steps in cheks now wait for resources, archive is no longer in the branch - it is pulled during the tests and release.
abfbb32 to
8705505
Compare
8705505 to
436a9cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Set up Helm | ||
| uses: azure/setup-helm@v4 | ||
| with: | ||
| version: v3.12.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is already included in the runner in newer version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will remove
Adding ability to get otel metrics from the kubernetes