From 774c29e47475737b1bed0ea06fb403aead74093f Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Tue, 15 Nov 2022 20:28:35 +0800 Subject: [PATCH] Remove `timeseries.instance` (#9565) * Remove timeseries.instance * Drop timeseries.instance on migration ... and fix system tests --- apmpackage/apm/changelog.yml | 3 + .../elasticsearch/ingest_pipeline/default.yml | 4 + .../internal_metrics/fields/fields.yml | 3 - .../apm_data_stream_migration.yml | 3 + changelogs/head.asciidoc | 1 + internal/model/metricset.go | 8 - internal/model/metricset_test.go | 6 +- systemtest/approvals.go | 1 - ...tPipelineDataStreamMigration.approved.json | 123 +++++++------ .../TestTransactionAggregation.approved.json | 165 +++++++++--------- ...ansactionAggregationShutdown.approved.json | 3 - .../aggregation/txmetrics/aggregator.go | 23 +-- .../aggregation/txmetrics/aggregator_test.go | 13 +- 13 files changed, 160 insertions(+), 196 deletions(-) diff --git a/apmpackage/apm/changelog.yml b/apmpackage/apm/changelog.yml index 75e5e6b1c06..5eff801d460 100644 --- a/apmpackage/apm/changelog.yml +++ b/apmpackage/apm/changelog.yml @@ -18,6 +18,9 @@ - description: Add mappings for `transaction.representative_count` and `span.representative_count` type: enhancement link: https://github.com/elastic/apm-server/pull/9458 + - description: Remove `timeseries.instance` field + type: enhancement + link: https://github.com/elastic/apm-server/pull/9565 - version: "8.5.0" changes: - description: Add package settings to enable the experimental collection of service metrics diff --git a/apmpackage/apm/data_stream/internal_metrics/elasticsearch/ingest_pipeline/default.yml b/apmpackage/apm/data_stream/internal_metrics/elasticsearch/ingest_pipeline/default.yml index ba9caf2c34c..92376dd8a70 100644 --- a/apmpackage/apm/data_stream/internal_metrics/elasticsearch/ingest_pipeline/default.yml +++ b/apmpackage/apm/data_stream/internal_metrics/elasticsearch/ingest_pipeline/default.yml @@ -16,3 +16,7 @@ processors: - remove: field: _metric_descriptions ignore_missing: true + - remove: + # Removed in 8.6.0 + field: timeseries.instance + ignore_missing: true diff --git a/apmpackage/apm/data_stream/internal_metrics/fields/fields.yml b/apmpackage/apm/data_stream/internal_metrics/fields/fields.yml index b5c75b9063e..a41223be022 100644 --- a/apmpackage/apm/data_stream/internal_metrics/fields/fields.yml +++ b/apmpackage/apm/data_stream/internal_metrics/fields/fields.yml @@ -126,9 +126,6 @@ type: keyword description: | Keyword of specific relevance in the service's domain (eg: 'db.postgresql.query', 'template.erb', 'cache', etc). -- name: timeseries.instance - type: keyword - description: Time series instance ID - name: transaction.failure_count type: long description: "Count of transactions with 'event.outcome: failure'" diff --git a/apmpackage/apm/data_stream/traces/elasticsearch/ingest_pipeline/apm_data_stream_migration.yml b/apmpackage/apm/data_stream/traces/elasticsearch/ingest_pipeline/apm_data_stream_migration.yml index 2874d12a0c8..a713b81b92e 100644 --- a/apmpackage/apm/data_stream/traces/elasticsearch/ingest_pipeline/apm_data_stream_migration.yml +++ b/apmpackage/apm/data_stream/traces/elasticsearch/ingest_pipeline/apm_data_stream_migration.yml @@ -37,3 +37,6 @@ processors: if: ctx.data_stream != null field: _index value: "{{data_stream.type}}-{{data_stream.dataset}}-{{data_stream.namespace}}" + - remove: + field: timeseries # remove timeseries.instance + ignore_missing: true diff --git a/changelogs/head.asciidoc b/changelogs/head.asciidoc index d3b19221990..dc05929198d 100644 --- a/changelogs/head.asciidoc +++ b/changelogs/head.asciidoc @@ -9,6 +9,7 @@ https://github.com/elastic/apm-server/compare/8.5\...main[View commits] - `ecs.version` is no longer added to document `_source`; it is added as a `constant_keyword` field {pull}9208[9208] - `context.http.response.*_size` fields now enforce integer values {pull}9429[9429] - `observer.id` and `observer.ephemeral_id` are no longer added to APM documents {pull}9412[9412] +- `timeseries.instance` has been removed from transaction metrics docs; it was never used {pull}9565[9565] [float] ==== Deprecations diff --git a/internal/model/metricset.go b/internal/model/metricset.go index 9ddd6b2d0ea..040eb0cfa48 100644 --- a/internal/model/metricset.go +++ b/internal/model/metricset.go @@ -49,11 +49,6 @@ type Metricset struct { // Samples holds the metrics in the set. Samples []MetricsetSample - // TimeseriesInstanceID holds an optional identifier for the timeseries - // instance, such as a hash of the labels used for aggregating the - // metrics. - TimeseriesInstanceID string - // Name holds an optional name for the metricset. Name string @@ -157,9 +152,6 @@ func (a *AggregatedDuration) fields() mapstr.M { } func (me *Metricset) setFields(fields *mapStr) { - if me.TimeseriesInstanceID != "" { - fields.set("timeseries", mapstr.M{"instance": me.TimeseriesInstanceID}) - } if me.DocCount > 0 { fields.set("_doc_count", me.DocCount) } diff --git a/internal/model/metricset_test.go b/internal/model/metricset_test.go index bc2057753e0..68fb28091d2 100644 --- a/internal/model/metricset_test.go +++ b/internal/model/metricset_test.go @@ -64,14 +64,12 @@ func TestMetricset(t *testing.T) { }, { Metricset: &Metricset{ - TimeseriesInstanceID: "foo", - DocCount: 6, + DocCount: 6, }, Output: mapstr.M{ - "timeseries": mapstr.M{"instance": "foo"}, "_doc_count": int64(6), }, - Msg: "Timeseries instance and _doc_count", + Msg: "_doc_count", }, { Metricset: &Metricset{ diff --git a/systemtest/approvals.go b/systemtest/approvals.go index 50e900fa9e6..da1b1644824 100644 --- a/systemtest/approvals.go +++ b/systemtest/approvals.go @@ -69,7 +69,6 @@ var apmEventSortFields = []string{ "transaction.id", "span.id", "error.id", - "timeseries.instance", "span.destination.service.resource", "transaction.type", "span.type", diff --git a/systemtest/approvals/TestIngestPipelineDataStreamMigration.approved.json b/systemtest/approvals/TestIngestPipelineDataStreamMigration.approved.json index e56d6f97ce0..476d1a47a97 100644 --- a/systemtest/approvals/TestIngestPipelineDataStreamMigration.approved.json +++ b/systemtest/approvals/TestIngestPipelineDataStreamMigration.approved.json @@ -259,6 +259,66 @@ } } }, + { + "@timestamp": "2022-09-12T03:52:00.000Z", + "agent": { + "name": "go" + }, + "container": { + "id": "spawn-6cd7c2e1-43d5-4c89-8480-e178b3bc70d1" + }, + "data_stream": { + "dataset": "apm.internal", + "namespace": "migrated", + "type": "metrics" + }, + "ecs": { + "version": "dynamic" + }, + "event": { + "agent_id_status": "missing", + "ingested": "dynamic", + "outcome": "failure" + }, + "host": { + "hostname": "corduroy", + "name": "corduroy" + }, + "metricset": { + "name": "transaction" + }, + "observer": { + "hostname": "dynamic", + "name": "instance-0000000000", + "type": "apm-server", + "version": "dynamic" + }, + "processor": { + "event": "metric", + "name": "metric" + }, + "service": { + "name": "main", + "node": { + "name": "spawn-6cd7c2e1-43d5-4c89-8480-e178b3bc70d1" + } + }, + "transaction": { + "duration": { + "histogram": { + "counts": [ + 1 + ], + "values": [ + 10239 + ] + } + }, + "name": "name", + "root": true, + "type": "type" + } + }, { "@timestamp": "2022-09-12T03:52:51.178Z", "agent": { @@ -465,69 +525,6 @@ } } }, - { - "@timestamp": "2022-09-12T03:52:00.000Z", - "agent": { - "name": "go" - }, - "container": { - "id": "spawn-6cd7c2e1-43d5-4c89-8480-e178b3bc70d1" - }, - "data_stream": { - "dataset": "apm.internal", - "namespace": "migrated", - "type": "metrics" - }, - "ecs": { - "version": "dynamic" - }, - "event": { - "agent_id_status": "missing", - "ingested": "dynamic", - "outcome": "failure" - }, - "host": { - "hostname": "corduroy", - "name": "corduroy" - }, - "metricset": { - "name": "transaction" - }, - "observer": { - "hostname": "dynamic", - "name": "instance-0000000000", - "type": "apm-server", - "version": "dynamic" - }, - "processor": { - "event": "metric", - "name": "metric" - }, - "service": { - "name": "main", - "node": { - "name": "spawn-6cd7c2e1-43d5-4c89-8480-e178b3bc70d1" - } - }, - "timeseries": { - "instance": "main:name:5b6121c8e3ec693e" - }, - "transaction": { - "duration": { - "histogram": { - "counts": [ - 1 - ], - "values": [ - 10239 - ] - } - }, - "name": "name", - "root": true, - "type": "type" - } - }, { "@timestamp": "2022-09-12T03:52:50.929Z", "agent": { diff --git a/systemtest/approvals/TestTransactionAggregation.approved.json b/systemtest/approvals/TestTransactionAggregation.approved.json index 1b6c2d4bbf4..e20d2155213 100644 --- a/systemtest/approvals/TestTransactionAggregation.approved.json +++ b/systemtest/approvals/TestTransactionAggregation.approved.json @@ -1,31 +1,9 @@ { "events": [ { - "@timestamp": "2021-09-15T20:11:06.000Z", + "@timestamp": "2006-01-02T15:04:05.000Z", "agent": { - "name": "elastic-node" - }, - "cloud": { - "account": { - "id": "account_id", - "name": "account_name" - }, - "availability_zone": "cloud_availability_zone", - "machine": { - "type": "machine_type" - }, - "project": { - "id": "project_id", - "name": "project_name" - }, - "provider": "cloud_provider", - "region": "cloud_region", - "service": { - "name": "lambda" - } - }, - "container": { - "id": "container-id" + "name": "go" }, "data_stream": { "dataset": "apm.internal", @@ -38,35 +16,18 @@ "event": { "agent_id_status": "missing", "ingested": "dynamic", - "outcome": "unknown" - }, - "faas": { - "coldstart": false, - "trigger": { - "type": "http" - } + "outcome": "success" }, "host": { - "hostname": "node-name", - "name": "node-name", + "hostname": "beowulf", + "name": "beowulf", "os": { - "platform": "darwin" - } - }, - "kubernetes": { - "pod": { - "name": "pod-name" + "platform": "minix" } }, - "labels": { - "tag1": "one" - }, "metricset": { "name": "transaction" }, - "numeric_labels": { - "tag2": 2 - }, "observer": { "hostname": "dynamic", "type": "apm-server", @@ -77,39 +38,33 @@ "name": "metric" }, "service": { - "environment": "staging", "language": { - "name": "ecmascript", - "version": "8" + "name": "go", + "version": "2.0" }, - "name": "1234_service-12a3", + "name": "systemtest", "node": { - "name": "node-123" + "name": "beowulf" }, "runtime": { - "name": "node", - "version": "8.0.0" - }, - "version": "5.1.3" - }, - "timeseries": { - "instance": "1234_service-12a3:faas:2a7054ba528b6bef" + "name": "gc", + "version": "2.0" + } }, "transaction": { "duration": { "histogram": { "counts": [ - 1 + 5 ], "values": [ - 38911 + 1003519 ] } }, - "name": "faas", - "result": "success", + "name": "abc", "root": true, - "type": "lambda" + "type": "backend" } }, { @@ -163,29 +118,48 @@ "version": "2.0" } }, - "timeseries": { - "instance": "systemtest:abc:954c62c5a2b8d0dd" - }, "transaction": { "duration": { "histogram": { "counts": [ - 5 + 10 ], "values": [ 1003519 ] } }, - "name": "abc", + "name": "def", "root": true, "type": "backend" } }, { - "@timestamp": "2006-01-02T15:04:05.000Z", + "@timestamp": "2021-09-15T20:11:06.000Z", "agent": { - "name": "go" + "name": "elastic-node" + }, + "cloud": { + "account": { + "id": "account_id", + "name": "account_name" + }, + "availability_zone": "cloud_availability_zone", + "machine": { + "type": "machine_type" + }, + "project": { + "id": "project_id", + "name": "project_name" + }, + "provider": "cloud_provider", + "region": "cloud_region", + "service": { + "name": "lambda" + } + }, + "container": { + "id": "container-id" }, "data_stream": { "dataset": "apm.internal", @@ -198,18 +172,35 @@ "event": { "agent_id_status": "missing", "ingested": "dynamic", - "outcome": "success" + "outcome": "unknown" + }, + "faas": { + "coldstart": false, + "trigger": { + "type": "http" + } }, "host": { - "hostname": "beowulf", - "name": "beowulf", + "hostname": "node-name", + "name": "node-name", "os": { - "platform": "minix" + "platform": "darwin" + } + }, + "kubernetes": { + "pod": { + "name": "pod-name" } }, + "labels": { + "tag1": "one" + }, "metricset": { "name": "transaction" }, + "numeric_labels": { + "tag2": 2 + }, "observer": { "hostname": "dynamic", "type": "apm-server", @@ -220,36 +211,36 @@ "name": "metric" }, "service": { + "environment": "staging", "language": { - "name": "go", - "version": "2.0" + "name": "ecmascript", + "version": "8" }, - "name": "systemtest", + "name": "1234_service-12a3", "node": { - "name": "beowulf" + "name": "node-123" }, "runtime": { - "name": "gc", - "version": "2.0" - } - }, - "timeseries": { - "instance": "systemtest:def:3f14f7166cf7659e" + "name": "node", + "version": "8.0.0" + }, + "version": "5.1.3" }, "transaction": { "duration": { "histogram": { "counts": [ - 10 + 1 ], "values": [ - 1003519 + 38911 ] } }, - "name": "def", + "name": "faas", + "result": "success", "root": true, - "type": "backend" + "type": "lambda" } } ] diff --git a/systemtest/approvals/TestTransactionAggregationShutdown.approved.json b/systemtest/approvals/TestTransactionAggregationShutdown.approved.json index b3c501c3f6d..fae42b05bce 100644 --- a/systemtest/approvals/TestTransactionAggregationShutdown.approved.json +++ b/systemtest/approvals/TestTransactionAggregationShutdown.approved.json @@ -51,9 +51,6 @@ "version": "2.0" } }, - "timeseries": { - "instance": "systemtest:name:942a34484a40cac3" - }, "transaction": { "duration": { "histogram": { diff --git a/x-pack/apm-server/aggregation/txmetrics/aggregator.go b/x-pack/apm-server/aggregation/txmetrics/aggregator.go index 04422298e7c..27aa8c0c75a 100644 --- a/x-pack/apm-server/aggregation/txmetrics/aggregator.go +++ b/x-pack/apm-server/aggregation/txmetrics/aggregator.go @@ -7,9 +7,7 @@ package txmetrics import ( "context" "encoding/binary" - "fmt" "math" - "strings" "sync" "sync/atomic" "time" @@ -222,7 +220,7 @@ func (a *Aggregator) publish(ctx context.Context) error { for hash, entries := range a.inactive.m { for _, entry := range entries { totalCount, counts, values := entry.transactionMetrics.histogramBuckets() - batch = append(batch, makeMetricset(entry.transactionAggregationKey, hash, totalCount, counts, values)) + batch = append(batch, makeMetricset(entry.transactionAggregationKey, totalCount, counts, values)) } delete(a.inactive.m, hash) } @@ -280,7 +278,7 @@ that configuration option appropriately, may lead to better results.`[1:], atomic.AddInt64(&a.metrics.overflowed, 1) counts := []int64{int64(math.Round(count))} values := []float64{float64(event.Event.Duration.Microseconds())} - return makeMetricset(key, hash, counts[0], counts, values) + return makeMetricset(key, counts[0], counts, values) } func (a *Aggregator) updateTransactionMetrics(key transactionAggregationKey, hash uint64, count float64, duration time.Duration) bool { @@ -391,17 +389,7 @@ func (a *Aggregator) makeTransactionAggregationKey(event model.APMEvent, interva } // makeMetricset makes a metricset event from key, counts, and values, with timestamp ts. -func makeMetricset( - key transactionAggregationKey, hash uint64, totalCount int64, counts []int64, values []float64, -) model.APMEvent { - // Record a timeseries instance ID, which should be uniquely identify the aggregation key. - var timeseriesInstanceID strings.Builder - timeseriesInstanceID.WriteString(key.serviceName) - timeseriesInstanceID.WriteRune(':') - timeseriesInstanceID.WriteString(key.transactionName) - timeseriesInstanceID.WriteRune(':') - timeseriesInstanceID.WriteString(fmt.Sprintf("%x", hash)) - +func makeMetricset(key transactionAggregationKey, totalCount int64, counts []int64, values []float64) model.APMEvent { return model.APMEvent{ Timestamp: key.timestamp, Agent: model.Agent{Name: key.agentName}, @@ -453,9 +441,8 @@ func makeMetricset( NumericLabels: key.AggregatedGlobalLabels.NumericLabels, Processor: model.MetricsetProcessor, Metricset: &model.Metricset{ - Name: metricsetName, - DocCount: totalCount, - TimeseriesInstanceID: timeseriesInstanceID.String(), + Name: metricsetName, + DocCount: totalCount, }, Transaction: &model.Transaction{ Name: key.transactionName, diff --git a/x-pack/apm-server/aggregation/txmetrics/aggregator_test.go b/x-pack/apm-server/aggregation/txmetrics/aggregator_test.go index 235c4411e43..edd3edfbd46 100644 --- a/x-pack/apm-server/aggregation/txmetrics/aggregator_test.go +++ b/x-pack/apm-server/aggregation/txmetrics/aggregator_test.go @@ -116,9 +116,8 @@ func TestProcessTransformablesOverflow(t *testing.T) { assert.Equal(t, model.APMEvent{ Processor: model.MetricsetProcessor, Metricset: &model.Metricset{ - Name: "transaction", - TimeseriesInstanceID: ":baz:9bf2d21a00716e4a", - DocCount: 1, + Name: "transaction", + DocCount: 1, }, Transaction: &model.Transaction{ Name: "baz", @@ -331,9 +330,8 @@ func TestAggregateRepresentativeCount(t *testing.T) { assert.Equal(t, model.APMEvent{ Processor: model.MetricsetProcessor, Metricset: &model.Metricset{ - Name: "transaction", - TimeseriesInstanceID: ":foo:9f7a6aa5754581fe", - DocCount: test.expectedCount, + Name: "transaction", + DocCount: test.expectedCount, }, Transaction: &model.Transaction{ Name: "foo", @@ -563,9 +561,6 @@ func TestAggregationFields(t *testing.T) { batch := expectBatch(t, batches) metricsets := batchMetricsets(t, batch) - for _, event := range metricsets { - event.Metricset.TimeseriesInstanceID = "" - } assert.ElementsMatch(t, expected, metricsets) }