Skip to content
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

Collects ClusterId for Telemetry Object Collector #1565

Merged
merged 15 commits into from
Feb 15, 2024

Conversation

salonichf5
Copy link
Contributor

@salonichf5 salonichf5 commented Feb 12, 2024

Proposed changes

  • Introduces a new field ClusterID for telemetry object.
  • Get the UID associated with kube-system namespace since it is unique and there is no concept of having a UID associated with cluster.

Problem: Collect ClusterID for unique identification of NGF installations in a cluster

Solution: We will fetch the UID for kube-system namspace and use that as the clusterID for recognizing different installations of NGF in a cluster since kube-system will always exists and chances of UID collision are rare.

Testing:

  1. Unit tests are updated
  2. Manual testing reflects the correct UID being updated
{"level":"debug","ts":"2024-02-09T19:53:09Z","logger":"telemetryJob","msg":"Gathering telemetry data"}
{"level":"debug","ts":"2024-02-09T19:53:09Z","logger":"telemetryJob","msg":"Exporting telemetry data"}
{"level":"debug","ts":"2024-02-09T19:53:09Z","logger":"telemetryExporter","msg":"Exporting telemetry","data":{"ProjectMetadata":{"Name":"NGF","Version":"edge"},"NodeCount":1,"NGFResourceCounts":{"Gateways":0,"GatewayClasses":1,"HTTPRoutes":0,"Secrets":0,"Services":0,"Endpoints":0,"Deployments":0},"ClusterID":"08bf9cfe-bb24-4764-b991-aa74a2ce0c2a"}}

Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide
specific feedback, add them here.

Closes #1303

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@github-actions github-actions bot added the enhancement New feature or request label Feb 12, 2024
internal/mode/static/state/dataplane/types.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Feb 13, 2024
go.mod Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
@salonichf5 salonichf5 marked this pull request as ready for review February 13, 2024 19:03
@salonichf5 salonichf5 requested a review from a team as a code owner February 13, 2024 19:03
@bjee19
Copy link
Contributor

bjee19 commented Feb 13, 2024

My PR on deployment replica count here is about to go in which will most likely cause some rebasing issues, I would prioritize getting that in then receiving feedback on this PR.

internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
@github-actions github-actions bot added the helm-chart Relates to helm chart label Feb 14, 2024
@github-actions github-actions bot removed dependencies Pull requests that update a dependency file helm-chart Relates to helm chart labels Feb 14, 2024
@sjberman
Copy link
Contributor

Be sure to update the PR description to include the issue to close.

Copy link
Contributor

@bjee19 bjee19 left a comment

Choose a reason for hiding this comment

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

Looking good, just a couple small things!

internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
salonichf5 and others added 4 commits February 15, 2024 10:41
Co-authored-by: Saylor Berman <s.berman@f5.com>
Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com>
Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com>
internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
salonichf5 and others added 3 commits February 15, 2024 11:58
Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com>
Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com>
Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com>
Copy link
Contributor

@bjee19 bjee19 left a comment

Choose a reason for hiding this comment

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

🚀 🚀 🚀 🚀 good job

@salonichf5 salonichf5 enabled auto-merge (squash) February 15, 2024 21:24
@salonichf5 salonichf5 merged commit 501f8f9 into nginxinc:main Feb 15, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Collect Cluster Identifier (NGF)
5 participants