From 9331523c8311600c6f02e8e6337d24ef13c0e7b6 Mon Sep 17 00:00:00 2001
From: Carson Ip <carson.ip@elastic.co>
Date: Mon, 12 Dec 2022 13:43:57 +0000
Subject: [PATCH] Add service metrics transaction.success_count

---
 apmpackage/apm/changelog.yml                    |  3 +++
 .../internal_metrics/fields/fields.yml          |  3 ---
 .../data_stream/internal_metrics/manifest.yml   |  6 +++++-
 .../model/modeldecoder/v2/transaction_test.go   |  2 ++
 internal/model/transaction.go                   | 17 +++++++----------
 .../TestServiceMetricsAggregation.approved.json | 10 ++++++++--
 .../aggregation/servicemetrics/aggregator.go    |  5 ++++-
 .../servicemetrics/aggregator_test.go           | 10 ++++++++--
 8 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/apmpackage/apm/changelog.yml b/apmpackage/apm/changelog.yml
index 72594f442bd..0d97df51bd9 100644
--- a/apmpackage/apm/changelog.yml
+++ b/apmpackage/apm/changelog.yml
@@ -3,6 +3,9 @@
     - description: Enable synthetic source for metrics data streams
       type: enhancement
       link: https://github.com/elastic/apm-server/pull/9756
+    - description: Change transaction.success_count type to aggregate_metric_double
+      type: enhancement
+      link: https://github.com/elastic/apm-server/pull/9791
 - version: "8.6.0"
   changes:
     - description: Change `ecs.version` to a `constant_keyword` field
diff --git a/apmpackage/apm/data_stream/internal_metrics/fields/fields.yml b/apmpackage/apm/data_stream/internal_metrics/fields/fields.yml
index a41223be022..535bf9d029c 100644
--- a/apmpackage/apm/data_stream/internal_metrics/fields/fields.yml
+++ b/apmpackage/apm/data_stream/internal_metrics/fields/fields.yml
@@ -129,9 +129,6 @@
 - name: transaction.failure_count
   type: long
   description: "Count of transactions with 'event.outcome: failure'"
-- name: transaction.success_count
-  type: long
-  description: "Count of transactions with 'event.outcome: success'"
 - name: transaction.duration.histogram
   type: histogram
   description: |
diff --git a/apmpackage/apm/data_stream/internal_metrics/manifest.yml b/apmpackage/apm/data_stream/internal_metrics/manifest.yml
index 2c3fd6d925d..c906da109d3 100644
--- a/apmpackage/apm/data_stream/internal_metrics/manifest.yml
+++ b/apmpackage/apm/data_stream/internal_metrics/manifest.yml
@@ -23,8 +23,12 @@ elasticsearch:
               properties:
                 summary:
                   type: aggregate_metric_double
-                  metrics: [sum, value_count]
+                  metrics: [ sum, value_count ]
                   default_metric: sum
+            success_count:
+              type: aggregate_metric_double
+              metrics: [ sum, value_count ]
+              default_metric: sum
     settings:
       index:
         sort.field: "@timestamp"
diff --git a/internal/model/modeldecoder/v2/transaction_test.go b/internal/model/modeldecoder/v2/transaction_test.go
index dba7c3b3dbb..0f854ccfb91 100644
--- a/internal/model/modeldecoder/v2/transaction_test.go
+++ b/internal/model/modeldecoder/v2/transaction_test.go
@@ -280,6 +280,8 @@ func TestDecodeMapToTransactionModel(t *testing.T) {
 				"DurationSummary.Sum",
 				"FailureCount",
 				"SuccessCount",
+				"SuccessCount.Count",
+				"SuccessCount.Sum",
 				"Root":
 				return true
 			}
diff --git a/internal/model/transaction.go b/internal/model/transaction.go
index 25db56f32c2..7cf6bc6de40 100644
--- a/internal/model/transaction.go
+++ b/internal/model/transaction.go
@@ -69,14 +69,11 @@ type Transaction struct {
 	// is subject to removal.
 	FailureCount int
 
-	// SuccessCount holds an aggregated count of transactions with the
-	// outcome "success". If SuccessCount is zero, it will be omitted from
-	// the output event.
-	//
-	// NOTE(axw) this is used only for service metrics, which are in technical
-	// preview. Do not use this field without discussion, as the field mapping
-	// is subject to removal.
-	SuccessCount int
+	// SuccessCount holds an aggregated count of transactions with different
+	// outcomes. A "failure" adds to the Count and a "success" adds to both
+	// the Count and the Sum. If Count is zero, it will be omitted from the
+	// output event.
+	SuccessCount SummaryMetric
 
 	Marks          TransactionMarks
 	Message        *Message
@@ -115,8 +112,8 @@ func (e *Transaction) fields() mapstr.M {
 	if e.FailureCount != 0 {
 		transaction.set("failure_count", e.FailureCount)
 	}
-	if e.SuccessCount != 0 {
-		transaction.set("success_count", e.SuccessCount)
+	if e.SuccessCount.Count != 0 {
+		transaction.maybeSetMapStr("success_count", e.SuccessCount.fields())
 	}
 	transaction.maybeSetString("name", e.Name)
 	transaction.maybeSetString("result", e.Result)
diff --git a/systemtest/approvals/TestServiceMetricsAggregation.approved.json b/systemtest/approvals/TestServiceMetricsAggregation.approved.json
index 56e8d501d06..714c1fd8a45 100644
--- a/systemtest/approvals/TestServiceMetricsAggregation.approved.json
+++ b/systemtest/approvals/TestServiceMetricsAggregation.approved.json
@@ -40,7 +40,10 @@
                         "value_count": 2
                     }
                 },
-                "success_count": 2,
+                "success_count": {
+                    "sum": 2,
+                    "value_count": 2
+                },
                 "type": "type1"
             }
         },
@@ -84,7 +87,10 @@
                         "value_count": 2
                     }
                 },
-                "success_count": 2,
+                "success_count": {
+                    "sum": 2,
+                    "value_count": 2
+                },
                 "type": "type2"
             }
         }
diff --git a/x-pack/apm-server/aggregation/servicemetrics/aggregator.go b/x-pack/apm-server/aggregation/servicemetrics/aggregator.go
index 52e2bb1a2d0..a5a56d69ce9 100644
--- a/x-pack/apm-server/aggregation/servicemetrics/aggregator.go
+++ b/x-pack/apm-server/aggregation/servicemetrics/aggregator.go
@@ -374,7 +374,10 @@ func makeMetricset(key aggregationKey, metrics serviceMetrics) model.APMEvent {
 		Transaction: &model.Transaction{
 			Type:         key.transactionType,
 			FailureCount: int(math.Round(metrics.failureCount)),
-			SuccessCount: int(math.Round(metrics.successCount)),
+			SuccessCount: model.SummaryMetric{
+				Count: int64(math.Round(metrics.successCount + metrics.failureCount)),
+				Sum:   metrics.successCount,
+			},
 			DurationSummary: model.SummaryMetric{
 				Count: metricCount,
 				Sum:   float64(time.Duration(math.Round(metrics.transactionDuration)).Microseconds()),
diff --git a/x-pack/apm-server/aggregation/servicemetrics/aggregator_test.go b/x-pack/apm-server/aggregation/servicemetrics/aggregator_test.go
index 81b75b6f5c6..f31dec18d0c 100644
--- a/x-pack/apm-server/aggregation/servicemetrics/aggregator_test.go
+++ b/x-pack/apm-server/aggregation/servicemetrics/aggregator_test.go
@@ -120,7 +120,10 @@ func TestAggregatorRun(t *testing.T) {
 				Count: 6,
 				Sum:   6000, // 6ms in micros
 			},
-			SuccessCount: 2,
+			SuccessCount: model.SummaryMetric{
+				Count: 5,
+				Sum:   2,
+			},
 			FailureCount: 3,
 		},
 	}, {
@@ -279,7 +282,10 @@ func TestAggregatorMaxGroups(t *testing.T) {
 					Count: 1,
 					Sum:   100000,
 				},
-				SuccessCount: 1,
+				SuccessCount: model.SummaryMetric{
+					Count: 1,
+					Sum:   1,
+				},
 			},
 			Metricset: &model.Metricset{Name: "service", DocCount: 1},
 		}, m)