Skip to content

Conversation

@elieserr
Copy link

@elieserr elieserr commented Aug 24, 2025

Added instrumentation for promethues compatible endpoints and otel.
Added tests and e2e tests.

I went forward this ways as I was trying to get familiar with codebase. Happy to go in any direction reviewers advise.

Checklist

Part of #965

@elieserr
Copy link
Author

Hi folks, trying to get some eyes here but not really sure who to tag, thanks for any pointer in advance
@JorTurFer @rickbrouwer @zroubalik tagging you 3 since helped me with previous unrelated PR kedacore/keda#6990

@JorTurFer
Copy link
Member

Thanks a lot for the contribution!

@elieserr elieserr force-pushed the instrument-http-add-on-operator branch from 723bbb9 to 176de5d Compare September 6, 2025 12:58
@elieserr elieserr force-pushed the instrument-http-add-on-operator branch from 176de5d to f56c1bd Compare September 23, 2025 20:17
@elieserr
Copy link
Author

I was able to reproduce the error in my local env. Was because of misconfig on go.mod. Also I rebased latest changes from main so this should work. lets see how e2e goes.

@zroubalik @wozniakjan @JorTurFer for workflow approval.

@elieserr
Copy link
Author

e2e went good on previous run, so fixing lints and some broken links from docs

@elieserr elieserr force-pushed the instrument-http-add-on-operator branch from bbad4f2 to bb94b8f Compare September 24, 2025 15:32
@wozniakjan wozniakjan requested a review from Copilot October 3, 2025 07:51
Copy link
Contributor

Copilot AI left a 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 PR adds instrumentation for Prometheus-compatible endpoints and OpenTelemetry (OTEL) metrics to the KEDA HTTP Add-on operator. The implementation provides monitoring capabilities for HTTP scaled objects through both metrics collection methods.

  • Added metrics collection infrastructure with Prometheus and OTEL exporters
  • Implemented HTTP scaled object count tracking in the operator controller
  • Added comprehensive end-to-end tests for both metrics collection methods

Reviewed Changes

Copilot reviewed 21 out of 22 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
operator/metrics/ New package containing metrics collection infrastructure with Prometheus and OTEL exporters
operator/controllers/http/httpscaledobject_controller.go Updated controller to track HTTP scaled object metrics with thread-safe map
operator/controllers/http/config/config.go Added metrics configuration structure with environment variable parsing
operator/main.go Integrated metrics collectors initialization into main function
tests/checks/operator_*_metrics/ Comprehensive E2E tests for both Prometheus and OTEL metrics
docs/ Updated documentation with metrics configuration instructions
config/ Added Kubernetes manifests for metrics service and E2E test configurations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

namespacedName := scaledObject.Name + scaledObject.Namespace
metricsData, ok := httpScaledObjectPromMetricsMap[namespacedName]
if ok {
metrics.RecordDeleteHTTPScaledObjectCount(scaledObject.Namespace)
Copy link

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

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

When the map entry already exists, the code calls RecordDeleteHTTPScaledObjectCount before RecordHTTPScaledObjectCount, which could lead to incorrect metrics if the namespace changed. This logic appears to be attempting to handle namespace changes, but it's unclear and potentially buggy.

Suggested change
metrics.RecordDeleteHTTPScaledObjectCount(scaledObject.Namespace)
// Record deletion for the old namespace before updating
metrics.RecordDeleteHTTPScaledObjectCount(metricsData.namespace)

Copilot uses AI. Check for mistakes.
@wozniakjan wozniakjan force-pushed the instrument-http-add-on-operator branch from e2ccb2c to af10bf6 Compare October 14, 2025 13:52
@wozniakjan wozniakjan force-pushed the instrument-http-add-on-operator branch from af10bf6 to ddaafca Compare October 21, 2025 07:38
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.

4 participants