Skip to content

feat: add configurable telemetry to ModelAsService CRD#3264

Draft
ishitasequeira wants to merge 2 commits intoopendatahub-io:mainfrom
ishitasequeira:feature/add-telemetry-to-maas
Draft

feat: add configurable telemetry to ModelAsService CRD#3264
ishitasequeira wants to merge 2 commits intoopendatahub-io:mainfrom
ishitasequeira:feature/add-telemetry-to-maas

Conversation

@ishitasequeira
Copy link
Contributor

Description

This PR adds configurable telemetry dimension controls to the ModelsAsService CRD. Platform operators can now enable or disable specific metric labels (user, organization, group, model) via spec.telemetry.metrics, allowing them to balance observability needs against metric cardinality and storage costs.

Key changes:

  • Extended ModelsAsServiceSpec with new Telemetry field containing MetricsConfig
  • Operator dynamically generates a TelemetryPolicy targeting the configured Gateway
  • Always-on dimensions (subscription, cost_center) ensure billing/chargeback works
  • High-cardinality dimensions like group default to disabled (opt-in only)

Example Usage:

apiVersion: components.opendatahub.io/v1alpha1
kind: ModelsAsService
metadata:
  name: default-modelsasservice
spec:
  gatewayRef:
    namespace: openshift-ingress
    name: maas-default-gateway
  telemetry:
    metrics:
      captureOrganization: true
      captureUser: false        # Disabled for GDPR compliance
      captureGroup: false       # High cardinality - keep disabled
      captureModelUsage: true

Generated TelemetryPolicy

apiVersion: extensions.kuadrant.io/v1alpha1
kind: TelemetryPolicy
metadata:
  name: maas-telemetry
  namespace: openshift-ingress
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: maas-default-gateway
  metrics:
    default:
      labels:
        # Always-on dimensions
        subscription: auth.identity.selected_subscription
        cost_center: auth.identity.costCenter
        tier: auth.identity.tier
        # Configurable dimensions
        organization_id: auth.identity.organizationId
        model: responseBodyJSON("/model")
        # user: omitted (captureUser: false)
        # group: omitted (captureGroup: false)

How Has This Been Tested?

Screenshot or short clip

Merge criteria

  • You have read the contributors guide.
  • Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
  • Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work
  • The developer has run the integration test pipeline and verified that it passed successfully

E2E test suite update requirement

When bringing new changes to the operator code, such changes are by default required to be accompanied by extending and/or updating the E2E test suite accordingly.

To opt-out of this requirement:

  1. Please inspect the opt-out guidelines, to determine if the nature of the PR changes allows for skipping this requirement
  2. If opt-out is applicable, provide justification in the dedicated E2E update requirement opt-out justification section below
  3. Check the checkbox below:
  • Skip requirement to update E2E test suite for this PR
  1. Submit/save these changes to the PR description. This will automatically trigger the check.

E2E update requirement opt-out justification

@openshift-ci
Copy link

openshift-ci bot commented Mar 12, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci
Copy link

openshift-ci bot commented Mar 12, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign asanzgom for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions
Copy link
Contributor

github-actions bot commented Mar 12, 2026

❌ E2E Update Requirement Check Failed

No e2e tests were added/updated as part of this PR.

Action required from the PR author: Please either:

  1. Add or update e2e tests relevant to the PR changes in the tests/e2e/ or Dockerfiles/e2e-tests/ directories, OR
  2. Evaluate the possibility of opting-out of this requirement:
    1. Inspect the opt-out guidelines, to determine if the nature of the PR changes allows for skipping this requirement
    2. If opt-out is applicable, please proceed to provide justification in the PR description (#### E2E update requirement opt-out justification section)
      • Note: In case your PR description does not adhere to our PR template and is missing the ### E2E test suite update requirement section (or any of its subsections), you can find the original PR template in .github/PULL_REQUEST_TEMPLATE.md, and restore the missing section from there
    3. After adding the justification, check the "Skip requirement to update E2E test suite for this PR" checkbox
    4. Submit/save changes to the PR description. The check will be triggered automatically and should pass

For more info, please refer to: workflow run details

Why this check exists:

  • E2E tests help ensure that changes work correctly in a real environment and don't break existing functionality
  • by default, every code-related PR should include an extension/update to the E2E test suite to cover the new changes
  • if the PR author is in doubt whether this requirement is applicable for their PR, please refer to the PR template for guidance about appropriate/inappropriate cases for opting-out

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 12, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: b5885ca8-086e-4c94-a479-77d66efc58a8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.

OpenGrep is compatible with Semgrep configurations. Add an opengrep.yml or semgrep.yml configuration file to your project to enable OpenGrep analysis.

@github-actions
Copy link
Contributor

This PR can't be merged just yet 😢

Please run make generate manifests api-docs and commit the changes.

For more info: https://github.com/opendatahub-io/opendatahub-operator/actions/runs/23003541042

@ishitasequeira ishitasequeira force-pushed the feature/add-telemetry-to-maas branch from f4e2c9a to 8ff183c Compare March 12, 2026 13:12
@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 94.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.59%. Comparing base (fa1fcdc) to head (bc784c4).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
...ents/modelsasservice/modelsasservice_controller.go 0.00% 2 Missing ⚠️
...elsasservice/modelsasservice_controller_actions.go 97.26% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3264      +/-   ##
==========================================
- Coverage   50.82%   50.59%   -0.23%     
==========================================
  Files         192      192              
  Lines       14022    14117      +95     
==========================================
+ Hits         7126     7143      +17     
- Misses       6155     6235      +80     
+ Partials      741      739       -2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

1 participant