Merge remote-tracking branch 'origin/main' into grafana-git-ui-sync

pkg/services/ngalert/prom/convert.go

@@ -1,16 +1,31 @@
 package prom
 
 import (
-	"encoding/json"
 	"fmt"
 	"time"
 
+	"github.com/google/uuid"
 	"gopkg.in/yaml.v3"
 
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
+	"github.com/grafana/grafana/pkg/util"
 )
 
+const (
+	// ruleUIDLabel is a special label that can be used to set a custom UID for a Prometheus
+	// alert rule when converting it to a Grafana alert rule. If this label is not present,
+	// a stable UID will be generated automatically based on the rule's data.
+	ruleUIDLabel = "__grafana_alert_rule_uid__"
+)
+
+const (
+	queryRefID          = "query"
+	prometheusMathRefID = "prometheus_math"
+	thresholdRefID      = "threshold"
+)
+
+// Config defines the configuration options for the Prometheus to Grafana rules converter.
 type Config struct {
 	DatasourceUID    string
 	DatasourceType   string
@@ -22,6 +37,7 @@ type Config struct {
 	AlertRules       RulesConfig
 }
 
+// RulesConfig contains configuration that applies to either recording or alerting rules.
 type RulesConfig struct {
 	IsPaused bool
 }
@@ -34,7 +50,7 @@ var (
 		FromTimeRange:    &defaultTimeRange,
 		EvaluationOffset: &defaultEvaluationOffset,
 		ExecErrState:     models.ErrorErrState,
-		NoDataState:      models.NoData,
+		NoDataState:      models.OK,
 	}
 )
 
@@ -42,6 +58,9 @@ type Converter struct {
 	cfg Config
 }
 
+// NewConverter creates a new Converter instance with the provided configuration.
+// It validates the configuration and returns an error if any required fields are missing
+// or if the configuration is invalid.
 func NewConverter(cfg Config) (*Converter, error) {
 	if cfg.DatasourceUID == "" {
 		return nil, fmt.Errorf("datasource UID is required")
@@ -114,6 +133,12 @@ func (p *Converter) convertRuleGroup(orgID int64, namespaceUID string, promGroup
 			gr.Title = fmt.Sprintf("%s (%d)", gr.Title, val)
 		}
 
+		uid, err := getUID(orgID, namespaceUID, promGroup.Name, i, rule)
+		if err != nil {
+			return nil, fmt.Errorf("failed to generate UID for rule '%s': %w", gr.Title, err)
+		}
+		gr.UID = uid
+
 		rules = append(rules, gr)
 	}
 
@@ -127,21 +152,52 @@ func (p *Converter) convertRuleGroup(orgID int64, namespaceUID string, promGroup
 	return result, nil
 }
 
+// getUID returns a UID for a Prometheus rule.
+// If the rule has a special label its value is used.
+// Otherwise, a stable UUID is generated by using a hash of the rule's data.
+func getUID(orgID int64, namespaceUID string, group string, position int, promRule PrometheusRule) (string, error) {
+	if uid, ok := promRule.Labels[ruleUIDLabel]; ok {
+		if err := util.ValidateUID(uid); err != nil {
+			return "", fmt.Errorf("invalid UID label value: %s; %w", uid, err)
+		}
+		return uid, nil
+	}
+
+	// Generate stable UUID based on the orgID, namespace, group and position.
+	uidData := fmt.Sprintf("%d|%s|%s|%d", orgID, namespaceUID, group, position)
+	u := uuid.NewSHA1(uuid.NameSpaceOID, []byte(uidData))
+
+	return u.String(), nil
+}
+
 func (p *Converter) convertRule(orgID int64, namespaceUID, group string, rule PrometheusRule) (models.AlertRule, error) {
 	var forInterval time.Duration
 	if rule.For != nil {
 		forInterval = time.Duration(*rule.For)
 	}
 
-	queryNode, err := createAlertQueryNode(p.cfg.DatasourceUID, p.cfg.DatasourceType, rule.Expr, *p.cfg.FromTimeRange, *p.cfg.EvaluationOffset)
+	var query []models.AlertQuery
+	var title string
+	var isPaused bool
+	var record *models.Record
+	var err error
+
+	isRecordingRule := rule.Record != ""
+	query, err = p.createQuery(rule.Expr, isRecordingRule)
 	if err != nil {
 		return models.AlertRule{}, err
 	}
 
-	var title string
-	if rule.Record != "" {
+	if isRecordingRule {
+		record = &models.Record{
+			From:   queryRefID,
+			Metric: rule.Record,
+		}
+
+		isPaused = p.cfg.RecordingRules.IsPaused
 		title = rule.Record
 	} else {
+		isPaused = p.cfg.AlertRules.IsPaused
 		title = rule.Alert
 	}
 
@@ -159,14 +215,16 @@ func (p *Converter) convertRule(orgID int64, namespaceUID, group string, rule Pr
 		OrgID:        orgID,
 		NamespaceUID: namespaceUID,
 		Title:        title,
-		Data:         []models.AlertQuery{queryNode},
-		Condition:    "A",
+		Data:         query,
+		Condition:    query[len(query)-1].RefID,
 		NoDataState:  p.cfg.NoDataState,
 		ExecErrState: p.cfg.ExecErrState,
 		Annotations:  rule.Annotations,
 		Labels:       labels,
 		For:          forInterval,
 		RuleGroup:    group,
+		IsPaused:     isPaused,
+		Record:       record,
 		Metadata: models.AlertRuleMetadata{
 			PrometheusStyleRule: &models.PrometheusStyleRule{
 				OriginalRuleDefinition: string(originalRuleDefinition),
@@ -174,47 +232,41 @@ func (p *Converter) convertRule(orgID int64, namespaceUID, group string, rule Pr
 		},
 	}
 
-	if rule.Record != "" {
-		result.Record = &models.Record{
-			From:   "A",
-			Metric: rule.Record,
-		}
-		result.IsPaused = p.cfg.RecordingRules.IsPaused
-	} else {
-		result.IsPaused = p.cfg.AlertRules.IsPaused
-	}
-
 	return result, nil
 }
 
-func createAlertQueryNode(datasourceUID, datasourceType, expr string, fromTimeRange, evaluationOffset time.Duration) (models.AlertQuery, error) {
-	modelData := map[string]interface{}{
-		"datasource": map[string]interface{}{
-			"type": datasourceType,
-			"uid":  datasourceUID,
-		},
-		"expr":    expr,
-		"instant": true,
-		"range":   false,
-		"refId":   "A",
-	}
-
-	if datasourceType == datasources.DS_LOKI {
-		modelData["queryType"] = "instant"
-	}
-
-	modelJSON, err := json.Marshal(modelData)
+// createQuery constructs the alert query nodes for a given Prometheus rule expression.
+// It returns a slice of AlertQuery that represent the evaluation steps for the rule.
+//
+// For recording rules it generates a single query node that
+// executes the PromQL query in the configured datasource.
+//
+// For alerting rules, it generates three query nodes:
+//  1. Query Node (query): Executes the PromQL query using the configured datasource.
+//  2. Math Node (prometheus_math): Applies a math expression "is_number($query) || is_nan($query) || is_inf($query)".
+//  3. Threshold Node (threshold): Gets the result from the math node and checks that it's greater than 0.
+//
+// This is needed to ensure that we keep the Prometheus behaviour, where any returned result
+// is considered alerting, and only when the query returns no data is the alert treated as normal.
+func (p *Converter) createQuery(expr string, isRecordingRule bool) ([]models.AlertQuery, error) {
+	queryNode, err := createQueryNode(p.cfg.DatasourceUID, p.cfg.DatasourceType, expr, *p.cfg.FromTimeRange, *p.cfg.EvaluationOffset)
 	if err != nil {
-		return models.AlertQuery{}, err
+		return nil, err
 	}
 
-	return models.AlertQuery{
-		DatasourceUID: datasourceUID,
-		Model:         modelJSON,
-		RefID:         "A",
-		RelativeTimeRange: models.RelativeTimeRange{
-			From: models.Duration(fromTimeRange + evaluationOffset),
-			To:   models.Duration(0 + evaluationOffset),
-		},
-	}, nil
+	if isRecordingRule {
+		return []models.AlertQuery{queryNode}, nil
+	}
+
+	mathNode, err := createMathNode()
+	if err != nil {
+		return nil, err
+	}
+
+	thresholdNode, err := createThresholdNode()
+	if err != nil {
+		return nil, err
+	}
+
+	return []models.AlertQuery{queryNode, mathNode, thresholdNode}, nil
 }

pkg/services/ngalert/prom/convert_test.go

@@ -1,15 +1,21 @@
 package prom
 
 import (
+	"encoding/json"
+	"fmt"
 	"testing"
 	"time"
 
+	"github.com/google/uuid"
 	prommodel "github.com/prometheus/common/model"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/yaml.v3"
 
+	"github.com/grafana/grafana/pkg/expr"
+	"github.com/grafana/grafana/pkg/expr/mathexp"
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
+	"github.com/grafana/grafana/pkg/util"
 )
 
 func TestPrometheusRulesToGrafana(t *testing.T) {
@@ -121,6 +127,9 @@ func TestPrometheusRulesToGrafana(t *testing.T) {
 
 				if promRule.Record != "" {
 					require.Equal(t, promRule.Record, grafanaRule.Title)
+					require.NotNil(t, grafanaRule.Record)
+					require.Equal(t, grafanaRule.Record.From, queryRefID)
+					require.Equal(t, promRule.Record, grafanaRule.Record.Metric)
 				} else {
 					require.Equal(t, promRule.Alert, grafanaRule.Title)
 				}
@@ -136,6 +145,10 @@ func TestPrometheusRulesToGrafana(t *testing.T) {
 					expectedLabels[k] = v
 				}
 
+				uidData := fmt.Sprintf("%d|%s|%s|%d", tc.orgID, tc.namespace, tc.promGroup.Name, j)
+				u := uuid.NewSHA1(uuid.NameSpaceOID, []byte(uidData))
+				require.Equal(t, u.String(), grafanaRule.UID, tc.name)
+
 				require.Equal(t, expectedLabels, grafanaRule.Labels, tc.name)
 				require.Equal(t, promRule.Annotations, grafanaRule.Annotations, tc.name)
 				require.Equal(t, models.Duration(0*time.Minute), grafanaRule.Data[0].RelativeTimeRange.To)
@@ -190,3 +203,221 @@ func TestPrometheusRulesToGrafanaWithDuplicateRuleNames(t *testing.T) {
 	require.Equal(t, "another alert", group.Rules[2].Title)
 	require.Equal(t, "alert (3)", group.Rules[3].Title)
 }
+
+func TestCreateMathNode(t *testing.T) {
+	node, err := createMathNode()
+	require.NoError(t, err)
+
+	require.Equal(t, expr.DatasourceUID, node.DatasourceUID)
+	require.Equal(t, string(expr.QueryTypeMath), node.QueryType)
+	require.Equal(t, "prometheus_math", node.RefID)
+
+	var model map[string]interface{}
+	err = json.Unmarshal(node.Model, &model)
+	require.NoError(t, err)
+
+	require.Equal(t, "prometheus_math", model["refId"])
+	require.Equal(t, string(expr.QueryTypeMath), model["type"])
+	require.Equal(t, "is_number($query) || is_nan($query) || is_inf($query)", model["expression"])
+
+	ds := model["datasource"].(map[string]interface{})
+	require.Equal(t, expr.DatasourceUID, ds["name"])
+	require.Equal(t, expr.DatasourceType, ds["type"])
+	require.Equal(t, expr.DatasourceUID, ds["uid"])
+}
+
+func TestCreateThresholdNode(t *testing.T) {
+	node, err := createThresholdNode()
+	require.NoError(t, err)
+
+	require.Equal(t, expr.DatasourceUID, node.DatasourceUID)
+	require.Equal(t, string(expr.QueryTypeThreshold), node.QueryType)
+	require.Equal(t, "threshold", node.RefID)
+
+	var model map[string]interface{}
+	err = json.Unmarshal(node.Model, &model)
+	require.NoError(t, err)
+
+	require.Equal(t, "threshold", model["refId"])
+	require.Equal(t, string(expr.QueryTypeThreshold), model["type"])
+
+	ds := model["datasource"].(map[string]interface{})
+	require.Equal(t, expr.DatasourceUID, ds["name"])
+	require.Equal(t, expr.DatasourceType, ds["type"])
+	require.Equal(t, expr.DatasourceUID, ds["uid"])
+
+	conditions := model["conditions"].([]interface{})
+	require.Len(t, conditions, 1)
+
+	condition := conditions[0].(map[string]interface{})
+	evaluator := condition["evaluator"].(map[string]interface{})
+	require.Equal(t, string(expr.ThresholdIsAbove), evaluator["type"])
+	require.Equal(t, []interface{}{float64(0)}, evaluator["params"])
+}
+
+func TestPrometheusRulesToGrafana_NodesInRules(t *testing.T) {
+	cfg := Config{
+		DatasourceUID:  "datasource-uid",
+		DatasourceType: datasources.DS_PROMETHEUS,
+	}
+	converter, err := NewConverter(cfg)
+	require.NoError(t, err)
+
+	t.Run("alert rule should have math and threshold nodes", func(t *testing.T) {
+		group := PrometheusRuleGroup{
+			Name: "test",
+			Rules: []PrometheusRule{
+				{
+					Alert: "alert1",
+					Expr:  "up == 0",
+				},
+			},
+		}
+
+		result, err := converter.PrometheusRulesToGrafana(1, "namespace", group)
+		require.NoError(t, err)
+		require.Len(t, result.Rules, 1)
+		require.Len(t, result.Rules[0].Data, 3)
+
+		// First node should be query
+		require.Equal(t, "query", result.Rules[0].Data[0].RefID)
+
+		// Second node should be math
+		require.Equal(t, "prometheus_math", result.Rules[0].Data[1].RefID)
+		require.Equal(t, string(expr.QueryTypeMath), result.Rules[0].Data[1].QueryType)
+		// Check that the math expression is valid
+		var model map[string]interface{}
+		err = json.Unmarshal(result.Rules[0].Data[1].Model, &model)
+		require.NoError(t, err)
+		require.Equal(t, "is_number($query) || is_nan($query) || is_inf($query)", model["expression"])
+		// The math expression should be parsed successfully
+		_, err = mathexp.New(model["expression"].(string))
+		require.NoError(t, err)
+
+		// Third node should be threshold
+		require.Equal(t, "threshold", result.Rules[0].Data[2].RefID)
+		require.Equal(t, string(expr.QueryTypeThreshold), result.Rules[0].Data[2].QueryType)
+	})
+
+	t.Run("recording rule should only have query node", func(t *testing.T) {
+		group := PrometheusRuleGroup{
+			Name: "test",
+			Rules: []PrometheusRule{
+				{
+					Record: "metric",
+					Expr:   "sum(rate(http_requests_total[5m]))",
+				},
+			},
+		}
+
+		result, err := converter.PrometheusRulesToGrafana(1, "namespace", group)
+		require.NoError(t, err)
+		require.Len(t, result.Rules, 1)
+		require.Len(t, result.Rules[0].Data, 1)
+
+		// Should only have query node
+		require.Equal(t, "query", result.Rules[0].Data[0].RefID)
+	})
+}
+
+func TestPrometheusRulesToGrafana_UID(t *testing.T) {
+	orgID := int64(1)
+	namespace := "some-namespace"
+
+	promGroup := PrometheusRuleGroup{
+		Name:     "test-group-1",
+		Interval: prommodel.Duration(10 * time.Second),
+		Rules: []PrometheusRule{
+			{
+				Alert: "alert-1",
+				Expr:  "cpu_usage > 80",
+				For:   util.Pointer(prommodel.Duration(5 * time.Minute)),
+				Labels: map[string]string{
+					"severity":   "critical",
+					ruleUIDLabel: "rule-uid-1",
+				},
+				Annotations: map[string]string{
+					"summary": "CPU usage is critical",
+				},
+			},
+		},
+	}
+
+	converter, err := NewConverter(Config{
+		DatasourceUID:  "datasource-uid",
+		DatasourceType: datasources.DS_PROMETHEUS,
+	})
+	require.NoError(t, err)
+
+	t.Run("if not specified, UID is generated based on the rule index", func(t *testing.T) {
+		grafanaGroup, err := converter.PrometheusRulesToGrafana(orgID, namespace, promGroup)
+		require.NoError(t, err)
+
+		firstUID := grafanaGroup.Rules[0].UID
+
+		// Convert again
+		grafanaGroup, err = converter.PrometheusRulesToGrafana(orgID, namespace, promGroup)
+		require.NoError(t, err)
+
+		secondUID := grafanaGroup.Rules[0].UID
+
+		// They must be equal
+		require.NotEmpty(t, firstUID)
+		require.Equal(t, firstUID, secondUID)
+	})
+
+	t.Run("if the special label is specified", func(t *testing.T) {
+		t.Run("and the label is valid it should be used", func(t *testing.T) {
+			orgID := int64(1)
+			namespace := "some-namespace"
+
+			converter, err := NewConverter(Config{
+				DatasourceUID:  "datasource-uid",
+				DatasourceType: datasources.DS_PROMETHEUS,
+			})
+			require.NoError(t, err)
+
+			promGroup.Rules[0].Labels[ruleUIDLabel] = "rule-uid-1"
+
+			grafanaGroup, err := converter.PrometheusRulesToGrafana(orgID, namespace, promGroup)
+			require.NoError(t, err)
+
+			require.Equal(t, "rule-uid-1", grafanaGroup.Rules[0].UID)
+		})
+
+		t.Run("and the label is invalid", func(t *testing.T) {
+			orgID := int64(1)
+			namespace := "some-namespace"
+
+			converter, err := NewConverter(Config{
+				DatasourceUID:  "datasource-uid",
+				DatasourceType: datasources.DS_PROMETHEUS,
+			})
+			require.NoError(t, err)
+
+			// create a string of 50 characters
+			promGroup.Rules[0].Labels[ruleUIDLabel] = "aaaabbbbccccddddeeeeffffgggghhhhiiiijjjjkkkkllllmm" // too long
+
+			grafanaGroup, err := converter.PrometheusRulesToGrafana(orgID, namespace, promGroup)
+			require.Errorf(t, err, "invalid UID label value")
+			require.Nil(t, grafanaGroup)
+		})
+
+		t.Run("and the label is empty", func(t *testing.T) {
+			orgID := int64(1)
+			namespace := "some-namespace"
+
+			converter, err := NewConverter(Config{
+				DatasourceUID:  "datasource-uid",
+				DatasourceType: datasources.DS_PROMETHEUS,
+			})
+			require.NoError(t, err)
+
+			promGroup.Rules[0].Labels[ruleUIDLabel] = ""
+
+			grafanaGroup, err := converter.PrometheusRulesToGrafana(orgID, namespace, promGroup)
+			require.Errorf(t, err, "invalid UID label value")
+			require.Nil(t, grafanaGroup)
+		})
+	})
+}

pkg/services/ngalert/prom/query.go

@@ -0,0 +1,127 @@
+package prom
+
+import (
+	"encoding/json"
+	"fmt"
+	"time"
+
+	"github.com/grafana/grafana/pkg/expr"
+	"github.com/grafana/grafana/pkg/services/datasources"
+	"github.com/grafana/grafana/pkg/services/ngalert/models"
+)
+
+type CommonQueryModel struct {
+	Datasource datasources.DataSource `json:"datasource"`
+	RefID      string                 `json:"refId"`
+	Type       expr.QueryType         `json:"type"`
+}
+
+func createQueryNode(datasourceUID, datasourceType, expr string, fromTimeRange, evaluationOffset time.Duration) (models.AlertQuery, error) {
+	modelData := map[string]interface{}{
+		"datasource": map[string]interface{}{
+			"type": datasourceType,
+			"uid":  datasourceUID,
+		},
+		"expr":    expr,
+		"instant": true,
+		"range":   false,
+		"refId":   queryRefID,
+	}
+
+	if datasourceType == datasources.DS_LOKI {
+		modelData["queryType"] = "instant"
+	}
+
+	modelJSON, err := json.Marshal(modelData)
+	if err != nil {
+		return models.AlertQuery{}, err
+	}
+
+	return models.AlertQuery{
+		DatasourceUID: datasourceUID,
+		Model:         modelJSON,
+		RefID:         queryRefID,
+		RelativeTimeRange: models.RelativeTimeRange{
+			From: models.Duration(fromTimeRange + evaluationOffset),
+			To:   models.Duration(0 + evaluationOffset),
+		},
+	}, nil
+}
+
+type MathQueryModel struct {
+	expr.MathQuery
+	CommonQueryModel
+}
+
+func createMathNode() (models.AlertQuery, error) {
+	ds, err := expr.DataSourceModelFromNodeType(expr.TypeCMDNode)
+	if err != nil {
+		return models.AlertQuery{}, err
+	}
+
+	model := MathQueryModel{
+		CommonQueryModel: CommonQueryModel{
+			Datasource: *ds,
+			RefID:      prometheusMathRefID,
+			Type:       expr.QueryTypeMath,
+		},
+		MathQuery: expr.MathQuery{
+			Expression: fmt.Sprintf("is_number($%[1]s) || is_nan($%[1]s) || is_inf($%[1]s)", queryRefID),
+		},
+	}
+
+	modelJSON, err := json.Marshal(model)
+	if err != nil {
+		return models.AlertQuery{}, err
+	}
+
+	return models.AlertQuery{
+		DatasourceUID: expr.DatasourceUID,
+		Model:         modelJSON,
+		RefID:         prometheusMathRefID,
+		QueryType:     string(model.Type),
+	}, nil
+}
+
+type ThresholdQueryModel struct {
+	expr.ThresholdQuery
+	CommonQueryModel
+}
+
+func createThresholdNode() (models.AlertQuery, error) {
+	ds, err := expr.DataSourceModelFromNodeType(expr.TypeCMDNode)
+	if err != nil {
+		return models.AlertQuery{}, err
+	}
+
+	model := ThresholdQueryModel{
+		CommonQueryModel: CommonQueryModel{
+			Datasource: *ds,
+			RefID:      thresholdRefID,
+			Type:       expr.QueryTypeThreshold,
+		},
+		ThresholdQuery: expr.ThresholdQuery{
+			Expression: prometheusMathRefID,
+			Conditions: []expr.ThresholdConditionJSON{
+				{
+					Evaluator: expr.ConditionEvalJSON{
+						Type:   expr.ThresholdIsAbove,
+						Params: []float64{0},
+					},
+				},
+			},
+		},
+	}
+
+	modelJSON, err := json.Marshal(model)
+	if err != nil {
+		return models.AlertQuery{}, err
+	}
+
+	return models.AlertQuery{
+		DatasourceUID: expr.DatasourceUID,
+		Model:         modelJSON,
+		RefID:         thresholdRefID,
+		QueryType:     string(model.Type),
+	}, nil
+}

public/app/features/dashboard-scene/scene/layout-tabs/TabItem.tsx

@@ -70,10 +70,6 @@ export class TabItem
     return new TabItems(items.filter((item) => item instanceof TabItem));
   }
 
-  public onChangeTab() {
-    this.getParentLayout().changeTab(this);
-  }
-
   public onChangeTitle(title: string) {
     this.setState({ title });
   }

public/app/features/dashboard-scene/scene/layout-tabs/TabItemRenderer.tsx

@@ -1,5 +1,7 @@
 import { useMemo } from 'react';
+import { useLocation } from 'react-router';
 
+import { locationUtil } from '@grafana/data';
 import { SceneComponentProps, sceneGraph } from '@grafana/scenes';
 import { Tab, useElementSelection } from '@grafana/ui';
 
@@ -12,29 +14,27 @@ export function TabItemRenderer({ model }: SceneComponentProps<TabItem>) {
   const { title, key } = model.useState();
   const isClone = useMemo(() => isClonedKey(key!), [key]);
   const parentLayout = model.getParentLayout();
-  const { currentTab } = parentLayout.useState();
+  const { tabs, currentTabIndex } = parentLayout.useState();
   const dashboard = getDashboardSceneFor(model);
   const { isEditing } = dashboard.useState();
   const titleInterpolated = sceneGraph.interpolate(model, title, undefined, 'text');
   const { isSelected, onSelect } = useElementSelection(key);
+  const myIndex = tabs.findIndex((tab) => tab === model);
+  const isActive = myIndex === currentTabIndex;
+  const location = useLocation();
+  const href = locationUtil.getUrlForPartial(location, { tab: myIndex });
 
   return (
     <Tab
       className={!isClone && isSelected ? 'dashboard-selected-element' : undefined}
       label={titleInterpolated}
-      active={model === currentTab}
+      active={isActive}
+      href={href}
       onPointerDown={(evt) => {
-        evt.stopPropagation();
-
-        if (isEditing) {
-          if (isClone) {
-            dashboard.state.editPane.clearSelection();
-          } else {
-            onSelect?.(evt);
-          }
+        if (isEditing && isActive && !isClone) {
+          evt.stopPropagation();
+          onSelect?.(evt);
         }
-
-        parentLayout.changeTab(model);
       }}
     />
   );

public/app/features/dashboard-scene/scene/layout-tabs/TabsLayoutManager.tsx

@@ -1,4 +1,10 @@
-import { SceneObjectBase, SceneObjectState, VizPanel } from '@grafana/scenes';
+import {
+  SceneObjectBase,
+  SceneObjectState,
+  SceneObjectUrlSyncConfig,
+  SceneObjectUrlValues,
+  VizPanel,
+} from '@grafana/scenes';
 import { t } from 'app/core/internationalization';
 
 import { DashboardLayoutManager } from '../types/DashboardLayoutManager';
@@ -9,7 +15,7 @@ import { TabsLayoutManagerRenderer } from './TabsLayoutManagerRenderer';
 
 interface TabsLayoutManagerState extends SceneObjectState {
   tabs: TabItem[];
-  currentTab: TabItem;
+  currentTabIndex: number;
 }
 
 export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> implements DashboardLayoutManager {
@@ -26,14 +32,42 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
     },
     id: 'tabs-layout',
     createFromLayout: TabsLayoutManager.createFromLayout,
-
     kind: 'TabsLayout',
   };
 
   public readonly descriptor = TabsLayoutManager.descriptor;
 
+  protected _urlSync = new SceneObjectUrlSyncConfig(this, { keys: ['tab'] });
+
+  public constructor(state: Partial<TabsLayoutManagerState>) {
+    super({
+      ...state,
+      tabs: state.tabs ?? [new TabItem()],
+      currentTabIndex: state.currentTabIndex ?? 0,
+    });
+  }
+
+  public getUrlState() {
+    return { tab: this.state.currentTabIndex.toString() };
+  }
+
+  public updateFromUrl(values: SceneObjectUrlValues) {
+    if (!values.tab) {
+      return;
+    }
+    if (typeof values.tab === 'string') {
+      this.setState({ currentTabIndex: parseInt(values.tab, 10) });
+    }
+  }
+
+  public getCurrentTab(): TabItem {
+    return this.state.tabs.length > this.state.currentTabIndex
+      ? this.state.tabs[this.state.currentTabIndex]
+      : this.state.tabs[0];
+  }
+
   public addPanel(vizPanel: VizPanel) {
-    this.state.currentTab.getLayout().addPanel(vizPanel);
+    this.getCurrentTab().getLayout().addPanel(vizPanel);
   }
 
   public getVizPanels(): VizPanel[] {
@@ -62,12 +96,12 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
   }
 
   public addNewRow() {
-    this.state.currentTab.getLayout().addNewRow();
+    this.getCurrentTab().getLayout().addNewRow();
   }
 
   public addNewTab() {
     const currentTab = new TabItem();
-    this.setState({ tabs: [...this.state.tabs, currentTab], currentTab });
+    this.setState({ tabs: [...this.state.tabs, currentTab], currentTabIndex: this.state.tabs.length });
   }
 
   public editModeChanged(isEditing: boolean) {
@@ -78,32 +112,33 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
     this.state.tabs.forEach((tab) => tab.getLayout().activateRepeaters?.());
   }
 
-  public removeTab(tab: TabItem) {
-    if (this.state.currentTab === tab) {
-      const currentTabIndex = this.state.tabs.indexOf(tab);
-      const nextTabIndex = currentTabIndex === 0 ? 1 : currentTabIndex - 1;
-      const nextTab = this.state.tabs[nextTabIndex];
-      this.setState({ tabs: this.state.tabs.filter((t) => t !== tab), currentTab: nextTab });
+  public removeTab(tabToRemove: TabItem) {
+    // Do not allow removing last tab (for now)
+    if (this.state.tabs.length === 1) {
       return;
     }
 
-    const filteredTab = this.state.tabs.filter((tab) => tab !== this.state.currentTab);
+    const currentTab = this.getCurrentTab();
+
+    if (currentTab === tabToRemove) {
+      const nextTabIndex = this.state.currentTabIndex > 0 ? this.state.currentTabIndex - 1 : 0;
+      this.setState({ tabs: this.state.tabs.filter((t) => t !== tabToRemove), currentTabIndex: nextTabIndex });
+      return;
+    }
+
+    const filteredTab = this.state.tabs.filter((tab) => tab !== tabToRemove);
     const tabs = filteredTab.length === 0 ? [new TabItem()] : filteredTab;
 
-    this.setState({ tabs, currentTab: tabs[tabs.length - 1] });
-  }
-
-  public changeTab(tab: TabItem) {
-    this.setState({ currentTab: tab });
+    this.setState({ tabs, currentTabIndex: 0 });
   }
 
   public static createEmpty(): TabsLayoutManager {
     const tab = new TabItem();
-    return new TabsLayoutManager({ tabs: [tab], currentTab: tab });
+    return new TabsLayoutManager({ tabs: [tab] });
   }
 
   public static createFromLayout(layout: DashboardLayoutManager): TabsLayoutManager {
     const tab = new TabItem({ layout: layout.clone() });
-    return new TabsLayoutManager({ tabs: [tab], currentTab: tab });
+    return new TabsLayoutManager({ tabs: [tab] });
   }
 }

public/app/features/dashboard-scene/scene/layout-tabs/TabsLayoutManagerRenderer.tsx

@@ -8,7 +8,8 @@ import { TabsLayoutManager } from './TabsLayoutManager';
 
 export function TabsLayoutManagerRenderer({ model }: SceneComponentProps<TabsLayoutManager>) {
   const styles = useStyles2(getStyles);
-  const { tabs, currentTab } = model.useState();
+  const { tabs, currentTabIndex } = model.useState();
+  const currentTab = tabs[currentTabIndex];
   const { layout } = currentTab.useState();
 
   return (
@@ -18,7 +19,9 @@ export function TabsLayoutManagerRenderer({ model }: SceneComponentProps<TabsLay
           <tab.Component model={tab} key={tab.state.key!} />
         ))}
       </TabsBar>
-      <TabContent className={styles.tabContentContainer}>{layout && <layout.Component model={layout} />}</TabContent>
+      <TabContent className={styles.tabContentContainer}>
+        {currentTab && <layout.Component model={layout} />}
+      </TabContent>
     </>
   );
 }

public/app/features/dashboard-scene/serialization/layoutSerializers/TabsLayoutSerializer.ts

@@ -44,6 +44,6 @@ export class TabsLayoutSerializer implements LayoutManagerSerializer {
         layout: layoutSerializerRegistry.get(layout.kind).serializer.deserialize(layout, elements, preload),
       });
     });
-    return new TabsLayoutManager({ tabs, currentTab: tabs[0] });
+    return new TabsLayoutManager({ tabs });
   }
 }

public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.test.ts

@@ -549,14 +549,7 @@ describe('dynamic layouts', () => {
       }),
     ];
 
-    const scene = setupDashboardScene(
-      getMinimalSceneState(
-        new TabsLayoutManager({
-          currentTab: tabs[0],
-          tabs,
-        })
-      )
-    );
+    const scene = setupDashboardScene(getMinimalSceneState(new TabsLayoutManager({ tabs })));
     const result = transformSceneToSaveModelSchemaV2(scene);
     expect(result.layout.kind).toBe('TabsLayout');
     const tabsLayout = result.layout.spec as TabsLayoutSpec;