Skip to content

[SREP-3287] dynatrace_enabled feature flag #466

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.

Sign up for GitHub

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

Open
aliceh wants to merge 4 commits into
base: master
Choose a base branch
from
+152 −38
Open

[SREP-3287] dynatrace_enabled feature flag #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b8ceddf
Added dynatrace_enabled flag rmo config
aliceh Feb 10, 2026
27fa0dd
Added docs/comments
aliceh Feb 10, 2026
77cc73e
Changed dynatrace-enabled flag to default to disabled
aliceh Feb 10, 2026
66ba512
Merge upstream/master and resolve conflicts
aliceh Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,32 @@ Currently the blackbox exporter deployment is only using the default config file

## Configuration

### Dynamic Configuration (ConfigMap)

For HostedCluster monitoring, the operator supports dynamic configuration via the `route-monitor-operator-config` ConfigMap in the `openshift-route-monitor-operator` namespace. This ConfigMap can be deployed per-region/per-sector using the template at `hack/olm-registry/rmo-config-template.yaml`.

**Supported ConfigMap fields:**
- `probe-api-url`: RHOBS synthetics API URL
- `probe-tenant`: RHOBS tenant name (default: "hcp")
- `oidc-client-id`: OIDC client ID for RHOBS authentication
- `oidc-client-secret`: OIDC client secret for RHOBS authentication
- `oidc-issuer-url`: OIDC issuer URL for RHOBS authentication
- `only-public-clusters`: Set to "true" to only monitor public clusters
- `dynatrace-enabled`: Enable/disable Dynatrace synthetic monitoring (default: "false")

Comment on lines +67 to +68
Copy link

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Dynatrace default behavior in docs is inverted.

This section documents Dynatrace as disabled by default (dynatrace-enabled default "false"), but the PR objective states Dynatrace should be enabled unless explicitly set to "false". Please align the field description and example accordingly.

Also applies to: 73-78

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 67 - 68, The README currently says
`dynatrace-enabled` defaults to "false" but the PR requires Dynatrace to be
enabled by default; update the field description for `dynatrace-enabled` to
indicate default: "true" and adjust any examples/usage blocks (referenced around
the same section lines and the example at lines 73-78) so they reflect that
Dynatrace is enabled unless explicitly set to "false" (change wording and
example values accordingly).

**Note:** ConfigMap values override command-line flags when present.

### Dynatrace Synthetic Monitoring

Dynatrace monitoring is **disabled by default**. To enable Dynatrace for specific sectors or regions:

```yaml
data:
dynatrace-enabled: "true"
```

This allows per-region/per-sector control of Dynatrace monitoring via the config template.

### Probe API URL (Experimental)

For RHOBS synthetics integration with HostedCluster monitoring, configure the probe API URL:
Expand Down
50 changes: 32 additions & 18 deletions controllers/hostedcontrolplane/hostedcontrolplane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
hypershiftv1beta1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
"github.com/openshift/route-monitor-operator/api/v1alpha1"
"github.com/openshift/route-monitor-operator/config"
"github.com/openshift/route-monitor-operator/pkg/dynatrace"
"github.com/openshift/route-monitor-operator/pkg/rhobs"
"github.com/openshift/route-monitor-operator/pkg/util/finalizer"
utilreconcile "github.com/openshift/route-monitor-operator/pkg/util/reconcile"
Expand Down Expand Up @@ -95,10 +96,10 @@ func NewHostedControlPlaneReconciler(mgr manager.Manager, rhobsConfig RHOBSConfi
}
}

// getRHOBSConfig reads RHOBS configuration from the ConfigMap at reconcile time.
// getRHOBSConfig reads RHOBS and Dynatrace configuration from the ConfigMap at reconcile time.
// If the ConfigMap doesn't exist or has empty values, it falls back to the command-line
// flags stored in r.RHOBSConfig.
func (r *HostedControlPlaneReconciler) getRHOBSConfig(ctx context.Context) RHOBSConfig {
func (r *HostedControlPlaneReconciler) getRHOBSConfig(ctx context.Context) (RHOBSConfig, DynatraceConfig) {
configMap := &corev1.ConfigMap{}
err := r.Get(ctx, types.NamespacedName{
Name: configMapName,
Expand All @@ -110,7 +111,7 @@ func (r *HostedControlPlaneReconciler) getRHOBSConfig(ctx context.Context) RHOBS
if !kerr.IsNotFound(err) {
logger.V(2).Info("Failed to read ConfigMap, using fallback config", "error", err.Error())
}
return r.RHOBSConfig
return r.RHOBSConfig, DynatraceConfig{Enabled: false}
Copy link

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Dynatrace failover/default logic is reversed from the feature contract.

Current logic disables Dynatrace when the ConfigMap is unreadable, missing, empty, or invalid. Per PR objective, Dynatrace should be enabled unless explicitly set to "false".

🐛 Proposed fix 
-		return r.RHOBSConfig, DynatraceConfig{Enabled: false}
+		return r.RHOBSConfig, DynatraceConfig{Enabled: true}
@@
-	// Read Dynatrace configuration - defaults to disabled
-	dynatraceConfig := DynatraceConfig{Enabled: false}
-	if strings.TrimSpace(configMap.Data["dynatrace-enabled"]) == "true" {
-		dynatraceConfig.Enabled = true
-	}
+	// Read Dynatrace configuration - defaults to enabled unless explicitly disabled
+	dynatraceConfig := DynatraceConfig{Enabled: true}
+	if strings.TrimSpace(configMap.Data["dynatrace-enabled"]) == "false" {
+		dynatraceConfig.Enabled = false
+	}

Also applies to: 138-142

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@controllers/hostedcontrolplane/hostedcontrolplane.go` at line 114, The
current return of DynatraceConfig{Enabled: false} in the hosted control plane
config code inappropriately disables Dynatrace when the ConfigMap is
unreadable/missing/invalid; change the default to DynatraceConfig{Enabled: true}
unless the ConfigMap explicitly sets the value to the string "false". Update the
code paths that currently return r.RHOBSConfig, DynatraceConfig{Enabled: false}
(and the analogous block around the 138-142 range) to return r.RHOBSConfig,
DynatraceConfig{Enabled: true} by default, and add explicit handling to parse
the ConfigMap value into DynatraceConfig.Enabled so only an explicit "false"
disables it; keep references to RHOBSConfig and DynatraceConfig to locate the
changes.

}

// Merge ConfigMap values with fallback defaults
Expand All @@ -134,7 +135,13 @@ func (r *HostedControlPlaneReconciler) getRHOBSConfig(ctx context.Context) RHOBS
cfg.OnlyPublicClusters = true
}

return cfg
// Read Dynatrace configuration - defaults to disabled
dynatraceConfig := DynatraceConfig{Enabled: false}
if strings.TrimSpace(configMap.Data["dynatrace-enabled"]) == "true" {
dynatraceConfig.Enabled = true
}

return cfg, dynatraceConfig
}

//+kubebuilder:rbac:groups=openshift.io,resources=hostedcontrolplanes,verbs=get;list;watch;create;update;patch;delete
Expand All @@ -148,7 +155,8 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R
defer log.Info("Finished reconciling HostedControlPlane")

// Get dynamic config from ConfigMap (with fallback to command-line flags)
rhobsConfig := r.getRHOBSConfig(ctx)
rhobsConfig, dynatraceConfig := r.getRHOBSConfig(ctx)
log.V(2).Info("Loaded configuration", "dynatrace_enabled", dynatraceConfig.Enabled)

// Fetch the HostedControlPlane instance
hostedcontrolplane := &hypershiftv1beta1.HostedControlPlane{}
Expand All @@ -162,24 +170,30 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R
return utilreconcile.RequeueWith(err)
}

//Create Dynatrace API client
dynatraceApiClient, err := r.NewDynatraceApiClient(ctx)
if err != nil {
// If RHOBS is configured, Dynatrace client creation failure is non-fatal
if rhobsConfig.ProbeAPIURL != "" {
log.Info("Dynatrace client creation failed, continuing with RHOBS-only monitoring", "error", err.Error())
dynatraceApiClient = nil
// Create Dynatrace API client only if Dynatrace is enabled
var dynatraceApiClient *dynatrace.DynatraceApiClient
if dynatraceConfig.Enabled {
client, err := r.NewDynatraceApiClient(ctx)
if err != nil {
// If RHOBS is configured, Dynatrace client creation failure is non-fatal
if rhobsConfig.ProbeAPIURL != "" {
log.Info("Dynatrace client creation failed, continuing with RHOBS-only monitoring", "error", err.Error())
} else {
log.Error(err, "failed to create dynatrace client")
return utilreconcile.RequeueWith(err)
}
} else {
log.Error(err, "failed to create dynatrace client")
return utilreconcile.RequeueWith(err)
dynatraceApiClient = client
}
} else {
log.V(2).Info("Dynatrace monitoring disabled via feature flag")
}

// If the HostedControlPlane is marked for deletion, clean up
shouldDelete := finalizer.WasDeleteRequested(hostedcontrolplane)
if shouldDelete {
// Only attempt Dynatrace deletion if client was successfully created
if dynatraceApiClient != nil {
// Only attempt Dynatrace deletion if Dynatrace is enabled and client was successfully created
if dynatraceConfig.Enabled && dynatraceApiClient != nil {
err = r.deleteDynatraceHttpMonitorResources(dynatraceApiClient, log, hostedcontrolplane)
if err != nil {
// If RHOBS is configured, Dynatrace failures are non-fatal - log warning and continue
Expand Down Expand Up @@ -289,8 +303,8 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R
return utilreconcile.RequeueWith(err)
}

// Only attempt Dynatrace deployment if client was successfully created
if dynatraceApiClient != nil {
// Only attempt Dynatrace deployment if Dynatrace is enabled and client was successfully created
if dynatraceConfig.Enabled && dynatraceApiClient != nil {
log.Info("Deploying HTTP Monitor Resources")
err = r.deployDynatraceHttpMonitorResources(ctx, dynatraceApiClient, log, hostedcontrolplane)
if err != nil {
Expand Down
96 changes: 77 additions & 19 deletions controllers/hostedcontrolplane/hostedcontrolplane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,16 +856,18 @@ func TestHostedControlPlaneReconciler_getRHOBSConfig(t *testing.T) {
}

tests := []struct {
name string
configMap *corev1.ConfigMap
fallbackConfig RHOBSConfig
expected RHOBSConfig
name string
configMap *corev1.ConfigMap
fallbackConfig RHOBSConfig
expectedRHOBS RHOBSConfig
expectedDynatrace DynatraceConfig
}{
{
name: "ConfigMap not present - uses fallback config",
configMap: nil,
fallbackConfig: fallbackConfig,
expected: fallbackConfig,
name: "ConfigMap not present - uses fallback config",
configMap: nil,
fallbackConfig: fallbackConfig,
expectedRHOBS: fallbackConfig,
expectedDynatrace: DynatraceConfig{Enabled: false}, // Default is disabled
},
{
name: "ConfigMap present with all values - uses ConfigMap values",
Expand All @@ -881,17 +883,19 @@ func TestHostedControlPlaneReconciler_getRHOBSConfig(t *testing.T) {
"oidc-client-secret": "configmap-secret",
"oidc-issuer-url": "https://configmap-issuer.example.com",
"only-public-clusters": "true",
"dynatrace-enabled": "true",
},
},
fallbackConfig: fallbackConfig,
expected: RHOBSConfig{
expectedRHOBS: RHOBSConfig{
ProbeAPIURL: "https://configmap-api.example.com/probes",
Tenant: "configmap-tenant",
OIDCClientID: "configmap-client-id",
OIDCClientSecret: "configmap-secret",
OIDCIssuerURL: "https://configmap-issuer.example.com",
OnlyPublicClusters: true,
},
expectedDynatrace: DynatraceConfig{Enabled: true},
},
{
name: "ConfigMap present with partial values - merges with fallback",
Expand All @@ -907,14 +911,15 @@ func TestHostedControlPlaneReconciler_getRHOBSConfig(t *testing.T) {
},
},
fallbackConfig: fallbackConfig,
expected: RHOBSConfig{
expectedRHOBS: RHOBSConfig{
ProbeAPIURL: "https://partial-api.example.com/probes",
Tenant: "partial-tenant",
OIDCClientID: "fallback-client-id",
OIDCClientSecret: "fallback-secret",
OIDCIssuerURL: "https://fallback-issuer.example.com",
OnlyPublicClusters: false,
},
expectedDynatrace: DynatraceConfig{Enabled: false}, // Default is disabled when not specified
},
{
name: "ConfigMap present with empty values - uses fallback for empty fields",
Expand All @@ -930,17 +935,19 @@ func TestHostedControlPlaneReconciler_getRHOBSConfig(t *testing.T) {
"oidc-client-secret": "",
"oidc-issuer-url": "",
"only-public-clusters": "false",
"dynatrace-enabled": "", // empty defaults to enabled
},
},
fallbackConfig: fallbackConfig,
expected: RHOBSConfig{
expectedRHOBS: RHOBSConfig{
ProbeAPIURL: "fallback-api.example.com/probes", // wrong, should be fallback
Tenant: "fallback-tenant", // empty/whitespace uses fallback
OIDCClientID: "valid-id",
OIDCClientSecret: "fallback-secret",
OIDCIssuerURL: "https://fallback-issuer.example.com",
OnlyPublicClusters: false,
},
expectedDynatrace: DynatraceConfig{Enabled: false}, // Empty defaults to disabled
},
{
name: "ConfigMap in wrong namespace - uses fallback config",
Expand All @@ -953,8 +960,9 @@ func TestHostedControlPlaneReconciler_getRHOBSConfig(t *testing.T) {
"probe-api-url": "https://wrong-namespace.example.com/probes",
},
},
fallbackConfig: fallbackConfig,
expected: fallbackConfig,
fallbackConfig: fallbackConfig,
expectedRHOBS: fallbackConfig,
expectedDynatrace: DynatraceConfig{Enabled: false}, // Default is disabled
},
{
name: "ConfigMap with wrong name - uses fallback config",
Expand All @@ -967,8 +975,54 @@ func TestHostedControlPlaneReconciler_getRHOBSConfig(t *testing.T) {
"probe-api-url": "https://wrong-name.example.com/probes",
},
},
fallbackConfig: fallbackConfig,
expected: fallbackConfig,
fallbackConfig: fallbackConfig,
expectedRHOBS: fallbackConfig,
expectedDynatrace: DynatraceConfig{Enabled: false}, // Default is disabled
},
{
name: "ConfigMap with Dynatrace enabled",
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: configMapName,
Namespace: config.OperatorNamespace,
},
Data: map[string]string{
"dynatrace-enabled": "true",
},
},
fallbackConfig: fallbackConfig,
expectedRHOBS: fallbackConfig,
expectedDynatrace: DynatraceConfig{Enabled: true},
},
{
name: "ConfigMap with Dynatrace disabled explicitly",
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: configMapName,
Namespace: config.OperatorNamespace,
},
Data: map[string]string{
"dynatrace-enabled": "false",
},
},
fallbackConfig: fallbackConfig,
expectedRHOBS: fallbackConfig,
expectedDynatrace: DynatraceConfig{Enabled: false},
},
{
name: "ConfigMap with Dynatrace set to invalid value - defaults to disabled",
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: configMapName,
Namespace: config.OperatorNamespace,
},
Data: map[string]string{
"dynatrace-enabled": "maybe",
},
},
fallbackConfig: fallbackConfig,
expectedRHOBS: fallbackConfig,
expectedDynatrace: DynatraceConfig{Enabled: false}, // Invalid value defaults to disabled
},
}

Expand All @@ -982,15 +1036,19 @@ func TestHostedControlPlaneReconciler_getRHOBSConfig(t *testing.T) {
r := newTestReconciler(t, objs...)
r.RHOBSConfig = tt.fallbackConfig

result := r.getRHOBSConfig(ctx)
rhobsResult, dynatraceResult := r.getRHOBSConfig(ctx)

// For the empty values test, we need to fix the expected value
if tt.name == "ConfigMap present with empty values - uses fallback for empty fields" {
tt.expected.ProbeAPIURL = "https://fallback-api.example.com/probes"
tt.expectedRHOBS.ProbeAPIURL = "https://fallback-api.example.com/probes"
}

if rhobsResult != tt.expectedRHOBS {
t.Errorf("getRHOBSConfig() RHOBS got = %+v, want = %+v", rhobsResult, tt.expectedRHOBS)
}

if result != tt.expected {
t.Errorf("getRHOBSConfig() got = %+v, want = %+v", result, tt.expected)
if dynatraceResult != tt.expectedDynatrace {
t.Errorf("getRHOBSConfig() Dynatrace got = %+v, want = %+v", dynatraceResult, tt.expectedDynatrace)
}
})
}
Expand Down
7 changes: 7 additions & 0 deletions controllers/hostedcontrolplane/synthetics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,13 @@ type RHOBSConfig struct {
OnlyPublicClusters bool
}

// DynatraceConfig holds Dynatrace feature flag configuration.
// Dynatrace monitoring is disabled by default and can be enabled per-sector/region
// by setting "dynatrace-enabled: true" in the ConfigMap.
type DynatraceConfig struct {
Enabled bool // Feature flag - defaults to false
}

// ensureRHOBSProbe ensures that a RHOBS probe exists for the HostedControlPlane
func (r *HostedControlPlaneReconciler) ensureRHOBSProbe(ctx context.Context, log logr.Logger, hostedcontrolplane *hypershiftv1beta1.HostedControlPlane, cfg RHOBSConfig) error {
clusterID := hostedcontrolplane.Spec.ClusterID
Expand Down
11 changes: 10 additions & 1 deletion hack/olm-registry/rmo-config-template.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ metadata:
#
# For production traffic splitting in high-load regions (e.g., us-east-1 with
# multiple RHOBS cells), see SREP-3225 for adding rhobs-cell labels to MCs.
#
# Configuration Parameters:
# - DYNATRACE_ENABLED: Controls Dynatrace synthetic monitoring (default: "false")
# Set to "true" to enable Dynatrace for specific sectors/regions
###############################################################################

parameters:
Expand All @@ -42,7 +46,11 @@ parameters:
required: true
- name: ONLY_PUBLIC_CLUSTERS
value: ""
required: false
required: false
- name: DYNATRACE_ENABLED
value: "false"
required: false
description: "Enable Dynatrace synthetic monitoring for this sector"

objects:
- apiVersion: hive.openshift.io/v1
Expand Down Expand Up @@ -79,6 +87,7 @@ objects:
oidc-client-secret: ${OIDC_CLIENT_SECRET}
oidc-issuer-url: ${OIDC_ISSUER_URL}
only-public-clusters: ${ONLY_PUBLIC_CLUSTERS}
dynatrace-enabled: ${DYNATRACE_ENABLED}
# Label the RMO namespace so the RHOBS MonitoringStack discovers
# ServiceMonitors in it and scrapes RMO metrics for the HCP tenant.
- apiVersion: v1
Expand Down