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

[feat] Adding Tracing(otel) to csi driver #309

Merged
merged 92 commits into from
Dec 16, 2024
Merged

Conversation

prajwalvathreya
Copy link
Contributor

@prajwalvathreya prajwalvathreya commented Nov 19, 2024

General:

This PR adds tracing to the csi driver controller and nodeserver.

  • Have you removed all sensitive information, including but not limited to access keys and passwords?
  • Have you checked to ensure there aren't other open or closed Pull Requests for the same bug/feature/question?

Pull Request Guidelines:

  1. Does your submission pass tests?
  2. Have you added tests?
  3. Are you addressing a single feature in this PR?
  4. Are your commits atomic, addressing one change per commit?
  5. Are you following the conventions of the language?
  6. Have you saved your large formatting changes for a different PR, so we can focus on your work?
  7. Have you explained your rationale for why this feature is needed?
  8. Have you linked your PR to an open issue

Copy link

codecov bot commented Nov 25, 2024

Codecov Report

Attention: Patch coverage is 33.94834% with 179 lines in your changes missing coverage. Please review.

Project coverage is 73.15%. Comparing base (75e2366) to head (a4cd2e6).

Files with missing lines Patch % Lines
pkg/observability/tracker.go 46.08% 58 Missing and 4 partials ⚠️
internal/driver/controllerserver_helper.go 11.76% 45 Missing and 15 partials ⚠️
internal/driver/controllerserver.go 21.42% 22 Missing ⚠️
internal/driver/nodeserver.go 18.51% 22 Missing ⚠️
internal/driver/server.go 55.00% 9 Missing ⚠️
main.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #309      +/-   ##
==========================================
- Coverage   76.57%   73.15%   -3.42%     
==========================================
  Files          22       23       +1     
  Lines        2403     2585     +182     
==========================================
+ Hits         1840     1891      +51     
- Misses        458      570     +112     
- Partials      105      124      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@prajwalvathreya prajwalvathreya marked this pull request as ready for review November 25, 2024 19:26
@prajwalvathreya prajwalvathreya requested review from a team as code owners November 25, 2024 19:26
@prajwalvathreya prajwalvathreya marked this pull request as draft November 25, 2024 19:27
.golangci.yml Show resolved Hide resolved
internal/driver/controllerserver_helper.go Show resolved Hide resolved
pkg/observability/tracker.go Show resolved Hide resolved
prajwalvathreya and others added 3 commits December 12, 2024 15:20
- changed jaeger-service to a load-balancer
- updated observability.md docs
- added notes on how to use functions in tracker.go
- added images
docs/observability.md Outdated Show resolved Hide resolved
```

### 2. Delete the Existing Release of the CSI Driver
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Include if they have it installed. Some people might not

hack/setup-tracing.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@komer3 komer3 left a comment

Choose a reason for hiding this comment

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

Great work on this! LGTM

@prajwalvathreya prajwalvathreya merged commit 2fe6257 into main Dec 16, 2024
7 of 9 checks passed
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