Skip to content

Commit

Permalink
Remove timeseries.instance
Browse files Browse the repository at this point in the history
  • Loading branch information
axw committed Nov 15, 2022
1 parent 5176087 commit 10dba5e
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 43 deletions.
3 changes: 3 additions & 0 deletions apmpackage/apm/changelog.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/TODO
- version: "8.5.0"
changes:
- description: Add package settings to enable the experimental collection of service metrics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,7 @@ processors:
- remove:
field: _metric_descriptions
ignore_missing: true
- remove:
# Removed in 8.6.0
field: timeseries.instance
ignore_missing: true
3 changes: 0 additions & 3 deletions apmpackage/apm/data_stream/internal_metrics/fields/fields.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
Expand Down
1 change: 1 addition & 0 deletions changelogs/head.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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}TODO[TODO]

[float]
==== Deprecations
Expand Down
8 changes: 0 additions & 8 deletions internal/model/metricset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
}
Expand Down
6 changes: 2 additions & 4 deletions internal/model/metricset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
1 change: 0 additions & 1 deletion systemtest/approvals.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ var apmEventSortFields = []string{
"transaction.id",
"span.id",
"error.id",
"timeseries.instance",
"span.destination.service.resource",
"transaction.type",
"span.type",
Expand Down
23 changes: 5 additions & 18 deletions x-pack/apm-server/aggregation/txmetrics/aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ package txmetrics
import (
"context"
"encoding/binary"
"fmt"
"math"
"strings"
"sync"
"sync/atomic"
"time"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 4 additions & 9 deletions x-pack/apm-server/aggregation/txmetrics/aggregator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}

Expand Down

0 comments on commit 10dba5e

Please sign in to comment.