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

COSI-46 expose metrics service in kubernetes and E2E test #72

Open
wants to merge 5 commits into
base: feature/COSI-19-add-s3-and-iam-metrics
Choose a base branch
from

Conversation

anurag4DSB
Copy link
Collaborator

@anurag4DSB anurag4DSB commented Dec 19, 2024

E2E tests are facing some issues, which I am debugging. But still reviewable.

This pull request introduces several changes to enable and configure metrics collection for the Scality COSI driver. The main changes include adding a new metrics service, updating deployment configurations to expose metrics, and modifying CI workflows to include metrics testing. This includes E2E tests

Metrics Collection Enablement:

  • New Metrics Service:

    • Added a new Service definition for the metrics endpoint in helm/scality-cosi-driver/templates/service.yaml.
    • Included the metrics service in the kustomize base and overlay configurations. [1] [2]
  • Deployment Configuration:

    • Updated helm/scality-cosi-driver/templates/deployment.yaml to include Prometheus annotations and set the metrics address. [1] [2]
    • Modified kustomize/base/deployment.yaml to include Prometheus annotations and set the metrics address. [1] [2]

CI Workflow Updates:

  • Metrics Testing Script:
    • Added a new script .github/scripts/e2e_tests_metrics_service.sh to test metrics collection during CI runs.
    • Updated .github/workflows/ci-e2e-tests.yml to include the new metrics testing script in the E2E tests.

RBAC Adjustments:

  • RBAC Rules:
    • Updated kustomize/base/rbac.yaml to include permissions for services and endpoints to support metrics collection.

These changes collectively enable the Scality COSI driver to expose metrics that can be scraped by Prometheus, ensuring better observability and monitoring.

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.62%. Comparing base (6701bdc) to head (7e74398).

Additional details and impacted files

Impacted file tree graph

Components Coverage Δ
🏠 Main Package ∅ <ø> (∅)
🚗 Driver Package 92.22% <ø> (ø)
📡 gRPC Factory Package 83.33% <ø> (ø)
🔐 IAM Client Package 100.00% <ø> (ø)
🌐 S3 Client Package 100.00% <ø> (ø)
🔧 Util Package 100.00% <ø> (ø)
📊 Metrics Package 100.00% <ø> (ø)
🔖 Constants Package ∅ <ø> (∅)
@@                           Coverage Diff                           @@
##           feature/COSI-19-add-s3-and-iam-metrics      #72   +/-   ##
=======================================================================
  Coverage                                   94.62%   94.62%           
=======================================================================
  Files                                          10       10           
  Lines                                         782      782           
=======================================================================
  Hits                                          740      740           
  Misses                                         36       36           
  Partials                                        6        6           

@anurag4DSB anurag4DSB force-pushed the feature/COSI-46-expose-metrics-service-in-kubernetes branch from 1e81b7e to e477885 Compare December 19, 2024 16:38
@anurag4DSB anurag4DSB marked this pull request as ready for review December 19, 2024 16:42
@anurag4DSB anurag4DSB changed the title Feature/cosi 46 expose metrics service in kubernetes COSI-46 expose metrics service in kubernetes and E2E test Dec 19, 2024
@anurag4DSB anurag4DSB force-pushed the feature/COSI-46-expose-metrics-service-in-kubernetes branch from b1e6d12 to 7e74398 Compare December 19, 2024 16:56

log_and_run curl -f http://localhost:$LOCAL_PORT/metrics > metrics_output.txt

if [ $? -eq 0 ]; then

Choose a reason for hiding this comment

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

I think you might lose the curl return code with the pipe | tee
https://unix.stackexchange.com/a/14276

Comment on lines +22 to +25
# Log command execution to the log file for debugging
log_and_run() {
"$@" 2>&1 | tee -a "$LOG_FILE"
}

Choose a reason for hiding this comment

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

Could this be replaced with the set -x option on bash

ports:
- protocol: TCP
port: {{ .Values.metrics.port }}
targetPort: {{ .Values.metrics.port }}
Copy link

@BourgoisMickael BourgoisMickael Dec 20, 2024

Choose a reason for hiding this comment

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

missing newline at end of some files

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