From c9b29e0be6ecbe80d0a1bc8b022d5c18cb42eac7 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 14 Dec 2020 18:33:23 +0800 Subject: [PATCH] model: sanitize/de-dot keys for labels, custom, and marks (#4465) (#4527) * model: sanitize label keys Replace reserved label key characters with '_', alleviating clients from having to do this. Also, optimise how we merge global and event-specific labels. Instead of adding all global labels and then using utility.DeepUpdate, de-dot the label keys first and range over the two maps to create a second map. Because the labels keys cannot have dots, we do not need to deep merge. * model: sanitize "custom" attribute keys Sanitize the keys of "custom" context using the same logic applied to labels. Also, drop the model.Custom type and just use common.MapStr. This eliminates some unnecessary pointer indirection. * model: sanitize "marks" keys * model/modeldecoder: drop label key regexp rules * Add changelog entry --- beater/beater_test.go | 5 +- docs/spec/rumv3/error.json | 25 ++---- docs/spec/rumv3/metadata.json | 19 ++--- docs/spec/rumv3/metricset.json | 19 ++--- docs/spec/rumv3/span.json | 19 ++--- docs/spec/rumv3/transaction.json | 85 +++++++------------ docs/spec/v2/error.json | 25 ++---- docs/spec/v2/metadata.json | 19 ++--- docs/spec/v2/metricset.json | 19 ++--- docs/spec/v2/span.json | 19 ++--- docs/spec/v2/transaction.json | 47 ++++------ model/context.go | 51 ++++------- model/error.go | 10 +-- model/error_test.go | 10 +-- model/labels.go | 79 +++++++++++++++++ model/metadata.go | 16 +--- model/metadata_test.go | 4 +- model/metricset.go | 5 +- model/metricset_test.go | 4 +- .../modeldecodertest/populator.go | 13 --- model/modeldecoder/rumv3/decoder.go | 12 +-- model/modeldecoder/rumv3/error_test.go | 4 +- model/modeldecoder/rumv3/model.go | 17 ++-- model/modeldecoder/rumv3/model_generated.go | 33 +------ model/modeldecoder/rumv3/model_test.go | 18 +--- model/modeldecoder/rumv3/transaction_test.go | 4 +- model/modeldecoder/v2/decoder.go | 12 +-- model/modeldecoder/v2/error_test.go | 4 +- model/modeldecoder/v2/model.go | 19 ++--- model/modeldecoder/v2/model_generated.go | 35 +------- model/modeldecoder/v2/model_test.go | 18 +--- model/modeldecoder/v2/transaction_test.go | 4 +- model/profile.go | 8 +- model/span.go | 4 +- model/span_test.go | 8 +- model/system_test.go | 2 +- model/transaction.go | 14 ++- model/transaction_test.go | 36 +++++++- processor/otel/consumer.go | 37 ++++---- 39 files changed, 337 insertions(+), 445 deletions(-) create mode 100644 model/labels.go diff --git a/beater/beater_test.go b/beater/beater_test.go index 4781ea4713e..be3fd8591ef 100644 --- a/beater/beater_test.go +++ b/beater/beater_test.go @@ -74,10 +74,9 @@ func setupBeater( // Add a label to test that everything // goes through the wrapped reporter. if tf.Labels == nil { - labels := make(model.Labels) - tf.Labels = &labels + tf.Labels = common.MapStr{} } - (*tf.Labels)["wrapped_reporter"] = true + tf.Labels["wrapped_reporter"] = true } } return origReporter(ctx, req) diff --git a/docs/spec/rumv3/error.json b/docs/spec/rumv3/error.json index d4ea5e9f6c5..ac58c10259e 100644 --- a/docs/spec/rumv3/error.json +++ b/docs/spec/rumv3/error.json @@ -14,11 +14,7 @@ "type": [ "null", "object" - ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": {} - } + ] }, "g": { "description": "Tags are a flat mapping of user-defined tags. Allowed value types are string, boolean and number values. Tags are indexed and searchable.", @@ -26,17 +22,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "p": { diff --git a/docs/spec/rumv3/metadata.json b/docs/spec/rumv3/metadata.json index 4fd9d8784d3..952a13292aa 100644 --- a/docs/spec/rumv3/metadata.json +++ b/docs/spec/rumv3/metadata.json @@ -8,17 +8,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "se": { diff --git a/docs/spec/rumv3/metricset.json b/docs/spec/rumv3/metricset.json index dcfd8dd3cbc..e148e024615 100644 --- a/docs/spec/rumv3/metricset.json +++ b/docs/spec/rumv3/metricset.json @@ -8,17 +8,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "sa": { diff --git a/docs/spec/rumv3/span.json b/docs/spec/rumv3/span.json index ed60a366ea6..9d5e0e364ed 100644 --- a/docs/spec/rumv3/span.json +++ b/docs/spec/rumv3/span.json @@ -76,17 +76,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "h": { diff --git a/docs/spec/rumv3/transaction.json b/docs/spec/rumv3/transaction.json index 66f7917899a..cdabc370ecb 100644 --- a/docs/spec/rumv3/transaction.json +++ b/docs/spec/rumv3/transaction.json @@ -14,11 +14,7 @@ "type": [ "null", "object" - ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": {} - } + ] }, "g": { "description": "Tags are a flat mapping of user-defined tags. Allowed value types are string, boolean and number values. Tags are indexed and searchable.", @@ -26,17 +22,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "p": { @@ -419,22 +412,16 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { + "additionalProperties": { + "type": [ + "null", + "object" + ], + "additionalProperties": { "type": [ "null", - "object" - ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "number" - ] - } - } + "number" + ] } } }, @@ -451,17 +438,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "sa": { @@ -723,17 +707,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "h": { diff --git a/docs/spec/v2/error.json b/docs/spec/v2/error.json index cf098edcf89..e02f2fbb41c 100644 --- a/docs/spec/v2/error.json +++ b/docs/spec/v2/error.json @@ -15,11 +15,7 @@ "type": [ "null", "object" - ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": {} - } + ] }, "experimental": { "description": "Experimental information is only processed when APM Server is started in development mode and should only be used by APM agent developers implementing new, unreleased features. The format is unspecified." @@ -516,17 +512,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "user": { diff --git a/docs/spec/v2/metadata.json b/docs/spec/v2/metadata.json index 6ec1566a096..7f164b12f66 100644 --- a/docs/spec/v2/metadata.json +++ b/docs/spec/v2/metadata.json @@ -133,17 +133,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "process": { diff --git a/docs/spec/v2/metricset.json b/docs/spec/v2/metricset.json index bbd73837086..09adf1e0ae1 100644 --- a/docs/spec/v2/metricset.json +++ b/docs/spec/v2/metricset.json @@ -55,17 +55,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "timestamp": { diff --git a/docs/spec/v2/span.json b/docs/spec/v2/span.json index d547b19efa2..77c6b57d6a6 100644 --- a/docs/spec/v2/span.json +++ b/docs/spec/v2/span.json @@ -459,17 +459,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } } } diff --git a/docs/spec/v2/transaction.json b/docs/spec/v2/transaction.json index 3ba95ab6ead..0d02d3d7372 100644 --- a/docs/spec/v2/transaction.json +++ b/docs/spec/v2/transaction.json @@ -14,11 +14,7 @@ "type": [ "null", "object" - ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": {} - } + ] }, "experimental": { "description": "Experimental information is only processed when APM Server is started in development mode and should only be used by APM agent developers implementing new, unreleased features. The format is unspecified." @@ -515,17 +511,14 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "string", - "boolean", - "number" - ], - "maxLength": 1024 - } + "additionalProperties": { + "type": [ + "null", + "string", + "boolean", + "number" + ], + "maxLength": 1024 } }, "user": { @@ -642,22 +635,16 @@ "null", "object" ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { + "additionalProperties": { + "type": [ + "null", + "object" + ], + "additionalProperties": { "type": [ "null", - "object" - ], - "additionalProperties": false, - "patternProperties": { - "^[^.*\"]*$": { - "type": [ - "null", - "number" - ] - } - } + "number" + ] } } }, diff --git a/model/context.go b/model/context.go index fac106c8ad4..ed4dd1ebb94 100644 --- a/model/context.go +++ b/model/context.go @@ -31,9 +31,9 @@ import ( type Context struct { Http *Http URL *URL - Labels *Labels + Labels common.MapStr Page *Page - Custom *Custom + Custom common.MapStr Message *Message Experimental interface{} } @@ -113,15 +113,6 @@ type Page struct { Referer *string } -// Labels holds user defined information nested under key tags -// -// TODO(axw) either get rid of this type, or use it consistently -// in all model types (looking at you, Metadata). -type Labels common.MapStr - -// Custom holds user defined information nested under key custom -type Custom common.MapStr - // Req bundles information related to an http request type Req struct { Method string @@ -205,28 +196,6 @@ func (page *Page) Fields() common.MapStr { return fields } -// Fields returns common.MapStr holding transformed data for attribute label. -func (labels *Labels) Fields() common.MapStr { - if labels == nil { - return nil - } - return common.MapStr(*labels) -} - -// Fields returns common.MapStr holding transformed data for attribute custom. -func (custom *Custom) Fields() common.MapStr { - if custom == nil { - return nil - } - // We use utility.Set to normalise decoded JSON types, - // e.g. json.Number is converted to a float64 if possible. - m := make(common.MapStr) - for k, v := range *custom { - utility.Set(m, k, v) - } - return m -} - func (req *Req) fields() common.MapStr { if req == nil { return nil @@ -288,3 +257,19 @@ func (s *Socket) fields() common.MapStr { utility.Set(fields, "remote_address", s.RemoteAddress) return fields } + +// customFields transforms in, returning a copy with sanitized keys +// and normalized field values, suitable for storing as "custom" +// in transaction and error documents.. +func customFields(in common.MapStr) common.MapStr { + if in == nil { + return nil + } + // We use utility.Set to normalise decoded JSON types, + // e.g. json.Number is converted to a float64 if possible. + out := make(common.MapStr, len(in)) + for k, v := range in { + utility.Set(out, sanitizeLabelKey(k), v) + } + return out +} diff --git a/model/error.go b/model/error.go index 0c247c1a3bf..11cffa61551 100644 --- a/model/error.go +++ b/model/error.go @@ -60,11 +60,11 @@ type Error struct { Metadata Metadata Culprit *string - Labels *Labels + Labels common.MapStr Page *Page HTTP *Http URL *URL - Custom *Custom + Custom common.MapStr Exception *Exception Log *Log @@ -125,11 +125,9 @@ func (e *Error) Transform(ctx context.Context, cfg *transform.Config) []beat.Eve } // first set the generic metadata (order is relevant) - e.Metadata.Set(fields) + e.Metadata.Set(fields, e.Labels) utility.Set(fields, "source", fields["client"]) // then add event specific information - // merges with metadata labels, overrides conflicting keys - utility.DeepUpdate(fields, "labels", e.Labels.Fields()) utility.Set(fields, "http", e.HTTP.Fields()) urlFields := e.URL.Fields() if urlFields != nil { @@ -178,7 +176,7 @@ func (e *Error) fields(ctx context.Context, cfg *transform.Config) common.MapStr e.updateCulprit(cfg) e.add("culprit", e.Culprit) - e.add("custom", e.Custom.Fields()) + e.add("custom", customFields(e.Custom)) e.add("grouping_key", e.calcGroupingKey(exceptionChain)) diff --git a/model/error_test.go b/model/error_test.go index 441c87a5f20..cf99375dbf0 100644 --- a/model/error_test.go +++ b/model/error_test.go @@ -282,8 +282,8 @@ func TestEvents(t *testing.T) { email, userIP, userAgent := "m@m.com", "127.0.0.1", "js-1.0" uid := "1234567889" url, referer := "https://localhost", "http://localhost" - labels := Labels(common.MapStr{"key": true}) - custom := Custom(common.MapStr{"foo": "bar"}) + labels := common.MapStr{"key": true} + custom := common.MapStr{"foo.bar": "baz"} serviceName, agentName, version := "myservice", "go", "1.0" md := Metadata{ @@ -362,9 +362,9 @@ func TestEvents(t *testing.T) { }, TransactionID: trID, TransactionSampled: &sampledTrue, - Labels: &labels, + Labels: labels, Page: &Page{URL: &URL{Original: &url}, Referer: &referer}, - Custom: &custom, + Custom: custom, RUM: true, }, @@ -380,7 +380,7 @@ func TestEvents(t *testing.T) { "user_agent": common.MapStr{"original": userAgent}, "error": common.MapStr{ "custom": common.MapStr{ - "foo": "bar", + "foo_bar": "baz", }, "grouping_key": "a61a65e048f403d9bcb2863d517fb48d", "log": common.MapStr{"message": "error log message"}, diff --git a/model/labels.go b/model/labels.go new file mode 100644 index 00000000000..de8c4dfa308 --- /dev/null +++ b/model/labels.go @@ -0,0 +1,79 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package model + +import ( + "encoding/json" + "strings" + + "github.com/elastic/beats/v7/libbeat/common" +) + +// If either or both globalLabels/eventLabels is non-empty, set a "labels" +// field in out to the combination of the labels. +// +// Label keys are sanitized, replacing the reserved characters '.', '*' and '"' +// with '_'. Event-specific labels take precedence over global labels. +// Null-valued labels are omitted. +func maybeSetLabels(out *mapStr, globalLabels, eventLabels common.MapStr) { + n := len(globalLabels) + len(eventLabels) + if n == 0 { + return + } + combined := make(common.MapStr, n) + for k, v := range globalLabels { + if v == nil { + continue + } + k := sanitizeLabelKey(k) + combined[k] = normalizeLabelValue(v) + } + for k, v := range eventLabels { + k := sanitizeLabelKey(k) + if v == nil { + delete(combined, k) + } else { + combined[k] = normalizeLabelValue(v) + } + } + out.set("labels", combined) +} + +// normalizeLabelValue transforms v into one of the accepted label value types: +// string, number, or boolean. +func normalizeLabelValue(v interface{}) interface{} { + switch v := v.(type) { + case json.Number: + if floatVal, err := v.Float64(); err == nil { + return common.Float(floatVal) + } + } + return v // types are guaranteed by decoders +} + +func sanitizeLabelKey(k string) string { + return strings.Map(replaceReservedLabelKeyRune, k) +} + +func replaceReservedLabelKeyRune(r rune) rune { + switch r { + case '.', '*', '"': + return '_' + } + return r +} diff --git a/model/metadata.go b/model/metadata.go index 9729cbf7e0c..099116d1fc7 100644 --- a/model/metadata.go +++ b/model/metadata.go @@ -19,8 +19,6 @@ package model import ( "github.com/elastic/beats/v7/libbeat/common" - - "github.com/elastic/apm-server/utility" ) type Metadata struct { @@ -34,7 +32,7 @@ type Metadata struct { Labels common.MapStr } -func (m *Metadata) Set(out common.MapStr) common.MapStr { +func (m *Metadata) Set(out common.MapStr, eventLabels common.MapStr) common.MapStr { fields := (*mapStr)(&out) fields.maybeSetMapStr("service", m.Service.Fields(m.System.Container.ID, m.System.name())) fields.maybeSetMapStr("agent", m.Service.AgentFields()) @@ -46,16 +44,6 @@ func (m *Metadata) Set(out common.MapStr) common.MapStr { fields.maybeSetMapStr("container", m.System.containerFields()) fields.maybeSetMapStr("kubernetes", m.System.kubernetesFields()) fields.maybeSetMapStr("cloud", m.Cloud.fields()) - if len(m.Labels) > 0 { - // These labels are merged with event-specific labels, - // hence we clone the map to avoid updating the shared - // metadata map. - // - // TODO(axw) we should only clone as needed or, better, - // avoid cloning altogether. For example, we could use - // DeepUpdateNoOverwrite in the other direction to copy - // the shared map into an event-specific labels map. - utility.Set(out, "labels", m.Labels) - } + maybeSetLabels(fields, m.Labels, eventLabels) return out } diff --git a/model/metadata_test.go b/model/metadata_test.go index 85646949dd8..bdb1e22936e 100644 --- a/model/metadata_test.go +++ b/model/metadata_test.go @@ -97,7 +97,7 @@ func TestMetadata_Set(t *testing.T) { "service": common.MapStr{"node": common.MapStr{"name": host}}}, }, } { - assert.Equal(t, test.output, test.input.Set(test.fields)) + assert.Equal(t, test.output, test.input.Set(test.fields, nil)) } } @@ -109,7 +109,7 @@ func BenchmarkMetadataSet(b *testing.B) { out := make(common.MapStr) for i := 0; i < b.N; i++ { - input.Set(out) + input.Set(out, nil) for k := range out { delete(out, k) } diff --git a/model/metricset.go b/model/metricset.go index cc70de05b43..00443bccfdc 100644 --- a/model/metricset.go +++ b/model/metricset.go @@ -162,7 +162,7 @@ func (me *Metricset) Transform(ctx context.Context, cfg *transform.Config) []bea } } - me.Metadata.Set(fields) + me.Metadata.Set(fields, me.Labels) var isInternal bool if eventFields := me.Event.fields(); eventFields != nil { @@ -178,9 +178,6 @@ func (me *Metricset) Transform(ctx context.Context, cfg *transform.Config) []bea utility.DeepUpdate(fields, metricsetSpanKey, spanFields) } - // merges with metadata labels, overrides conflicting keys - utility.DeepUpdate(fields, "labels", me.Labels) - if me.TimeseriesInstanceID != "" { fields["timeseries"] = common.MapStr{"instance": me.TimeseriesInstanceID} } diff --git a/model/metricset_test.go b/model/metricset_test.go index 33f55abe7a6..3a95eb46a4a 100644 --- a/model/metricset_test.go +++ b/model/metricset_test.go @@ -75,7 +75,7 @@ func TestTransform(t *testing.T) { { Metricset: &Metricset{ Metadata: metadata, - Labels: common.MapStr{"a.b": "a.b.value"}, + Labels: common.MapStr{"a_b": "a.b.value"}, Timestamp: timestamp, Samples: []Sample{ { @@ -94,7 +94,7 @@ func TestTransform(t *testing.T) { "data_stream.dataset": "apm.myservice", "processor": common.MapStr{"event": "metric", "name": "metric"}, "service": common.MapStr{"name": "myservice"}, - "labels": common.MapStr{"a.b": "a.b.value"}, + "labels": common.MapStr{"a_b": "a.b.value"}, "a": common.MapStr{"counter": float64(612)}, "some": common.MapStr{"gauge": float64(9.16)}, diff --git a/model/modeldecoder/modeldecodertest/populator.go b/model/modeldecoder/modeldecodertest/populator.go index f0923353f65..987b309a0c5 100644 --- a/model/modeldecoder/modeldecodertest/populator.go +++ b/model/modeldecoder/modeldecodertest/populator.go @@ -30,7 +30,6 @@ import ( "github.com/elastic/beats/v7/libbeat/common" - "github.com/elastic/apm-server/model" "github.com/elastic/apm-server/model/modeldecoder/nullable" ) @@ -240,18 +239,6 @@ func AssertStructValues(t *testing.T, i interface{}, isException func(string) bo m.Put(fmt.Sprintf("%s%v", values.Str, i), values.Str) } newVal = m - case *model.Labels: - m := model.Labels{} - for i := 0; i < values.N; i++ { - m[fmt.Sprintf("%s%v", values.Str, i)] = values.Str - } - newVal = &m - case *model.Custom: - m := model.Custom{} - for i := 0; i < values.N; i++ { - m[fmt.Sprintf("%s%v", values.Str, i)] = values.Str - } - newVal = &m case []string: m := make([]string, values.N) for i := 0; i < values.N; i++ { diff --git a/model/modeldecoder/rumv3/decoder.go b/model/modeldecoder/rumv3/decoder.go index 986b7f37ba5..13e9ca45d2e 100644 --- a/model/modeldecoder/rumv3/decoder.go +++ b/model/modeldecoder/rumv3/decoder.go @@ -208,8 +208,7 @@ func mapToErrorModel(from *errorEvent, metadata *model.Metadata, reqTime time.Ti if from.Context.IsSet() { // metadata labels and context labels are merged only in the output model if len(from.Context.Tags) > 0 { - labels := model.Labels(from.Context.Tags.Clone()) - out.Labels = &labels + out.Labels = from.Context.Tags.Clone() } if from.Context.Page.IsSet() { out.Page = &model.Page{} @@ -231,8 +230,7 @@ func mapToErrorModel(from *errorEvent, metadata *model.Metadata, reqTime time.Ti mapToResponseModel(from.Context.Response, out.HTTP.Response) } if len(from.Context.Custom) > 0 { - custom := model.Custom(from.Context.Custom.Clone()) - out.Custom = &custom + out.Custom = from.Context.Custom.Clone() } } if from.Culprit.IsSet() { @@ -729,13 +727,11 @@ func mapToTransactionModel(from *transaction, metadata *model.Metadata, reqTime if from.Context.IsSet() { if len(from.Context.Custom) > 0 { - custom := model.Custom(from.Context.Custom.Clone()) - out.Custom = &custom + out.Custom = from.Context.Custom.Clone() } // metadata labels and context labels are merged when transforming the output model if len(from.Context.Tags) > 0 { - labels := model.Labels(from.Context.Tags.Clone()) - out.Labels = &labels + out.Labels = from.Context.Tags.Clone() } if from.Context.Page.IsSet() { out.Page = &model.Page{} diff --git a/model/modeldecoder/rumv3/error_test.go b/model/modeldecoder/rumv3/error_test.go index 2e725294ecc..f8708d60af2 100644 --- a/model/modeldecoder/rumv3/error_test.go +++ b/model/modeldecoder/rumv3/error_test.go @@ -102,9 +102,9 @@ func TestDecodeMapToErrorModel(t *testing.T) { assert.Equal(t, ip, out.Metadata.Client.IP, out.Metadata.Client.IP.String()) // metadata labels and event labels should not be merged mLabels := common.MapStr{"init0": "init", "init1": "init", "init2": "init"} - tLabels := model.Labels{"overwritten0": "overwritten", "overwritten1": "overwritten"} + tLabels := common.MapStr{"overwritten0": "overwritten", "overwritten1": "overwritten"} assert.Equal(t, mLabels, out.Metadata.Labels) - assert.Equal(t, &tLabels, out.Labels) + assert.Equal(t, tLabels, out.Labels) // service and user values should be set modeldecodertest.AssertStructValues(t, &out.Metadata.Service, metadataExceptions("Node", "Agent.EphemeralID"), otherVal) modeldecodertest.AssertStructValues(t, &out.Metadata.User, metadataExceptions(), otherVal) diff --git a/model/modeldecoder/rumv3/model.go b/model/modeldecoder/rumv3/model.go index 72dba0cd7ce..f00bd5f870e 100644 --- a/model/modeldecoder/rumv3/model.go +++ b/model/modeldecoder/rumv3/model.go @@ -26,8 +26,7 @@ import ( ) var ( - patternAlphaNumericExt = `^[a-zA-Z0-9 _-]+$` - patternNoDotAsteriskQuote = `^[^.*"]*$` //do not allow '.' '*' '"' + patternAlphaNumericExt = `^[a-zA-Z0-9 _-]+$` enumOutcome = []string{"success", "failure", "unknown"} ) @@ -60,7 +59,7 @@ type context struct { // Custom can contain additional metadata to be stored with the event. // The format is unspecified and can be deeply nested objects. // The information will not be indexed or searchable in Elasticsearch. - Custom common.MapStr `json:"cu" validate:"patternKeys=patternNoDotAsteriskQuote"` + Custom common.MapStr `json:"cu"` // Page holds information related to the current page and page referers. // It is only sent from RUM agents. Page contextPage `json:"p"` @@ -76,7 +75,7 @@ type context struct { Service contextService `json:"se"` // Tags are a flat mapping of user-defined tags. Allowed value types are // string, boolean and number values. Tags are indexed and searchable. - Tags common.MapStr `json:"g" validate:"patternKeys=patternNoDotAsteriskQuote,inputTypesVals=string;bool;number,maxLengthVals=1024"` + Tags common.MapStr `json:"g" validate:"inputTypesVals=string;bool;number,maxLengthVals=1024"` // User holds information about the correlated user for this event. If // user data are provided here, all user related information from metadata // is ignored, otherwise the metadata's user information will be stored @@ -248,7 +247,7 @@ type errorTransactionRef struct { type metadata struct { // Labels are a flat mapping of user-defined tags. Allowed value types are // string, boolean and number values. Labels are indexed and searchable. - Labels common.MapStr `json:"l" validate:"patternKeys=patternNoDotAsteriskQuote,inputTypesVals=string;bool;number,maxLengthVals=1024"` + Labels common.MapStr `json:"l" validate:"inputTypesVals=string;bool;number,maxLengthVals=1024"` // Service metadata about the monitored service. Service metadataService `json:"se" validate:"required"` // User metadata, which can be overwritten on a per event basis. @@ -311,7 +310,7 @@ type metricset struct { Span metricsetSpanRef `json:"y"` // Tags are a flat mapping of user-defined tags. Allowed value types are // string, boolean and number values. Tags are indexed and searchable. - Tags common.MapStr `json:"g" validate:"patternKeys=patternNoDotAsteriskQuote,inputTypesVals=string;bool;number,maxLengthVals=1024"` + Tags common.MapStr `json:"g" validate:"inputTypesVals=string;bool;number,maxLengthVals=1024"` } type metricsetSamples struct { @@ -399,7 +398,7 @@ type spanContext struct { Service spanContextService `json:"se"` // Tags are a flat mapping of user-defined tags. Allowed value types are // string, boolean and number values. Tags are indexed and searchable. - Tags common.MapStr `json:"g" validate:"patternKeys=patternNoDotAsteriskQuote,inputTypesVals=string;bool;number,maxLengthVals=1024"` + Tags common.MapStr `json:"g" validate:"inputTypesVals=string;bool;number,maxLengthVals=1024"` } type spanContextDestination struct { @@ -532,7 +531,7 @@ type transaction struct { } type transactionMarks struct { - Events map[string]transactionMarkEvents `json:"-" validate:"patternKeys=patternNoDotAsteriskQuote"` + Events map[string]transactionMarkEvents `json:"-"` } var markEventsLongNames = map[string]string{ @@ -557,7 +556,7 @@ func (m *transactionMarks) UnmarshalJSON(data []byte) error { } type transactionMarkEvents struct { - Measurements map[string]float64 `json:"-" validate:"patternKeys=patternNoDotAsteriskQuote"` + Measurements map[string]float64 `json:"-"` } func (m *transactionMarkEvents) UnmarshalJSON(data []byte) error { diff --git a/model/modeldecoder/rumv3/model_generated.go b/model/modeldecoder/rumv3/model_generated.go index 53dd7710d5b..f423f4a73cb 100644 --- a/model/modeldecoder/rumv3/model_generated.go +++ b/model/modeldecoder/rumv3/model_generated.go @@ -29,8 +29,7 @@ import ( ) var ( - patternAlphaNumericExtRegexp = regexp.MustCompile(patternAlphaNumericExt) - patternNoDotAsteriskQuoteRegexp = regexp.MustCompile(patternNoDotAsteriskQuote) + patternAlphaNumericExtRegexp = regexp.MustCompile(patternAlphaNumericExt) ) func (val *metadataRoot) IsSet() bool { @@ -68,9 +67,6 @@ func (val *metadata) validate() error { return nil } for k, v := range val.Labels { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'l': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } switch t := v.(type) { case nil: case string: @@ -405,11 +401,6 @@ func (val *context) validate() error { if !val.IsSet() { return nil } - for k := range val.Custom { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'cu': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } - } if err := val.Page.validate(); err != nil { return errors.Wrapf(err, "p") } @@ -423,9 +414,6 @@ func (val *context) validate() error { return errors.Wrapf(err, "se") } for k, v := range val.Tags { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'g': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } switch t := v.(type) { case nil: case string: @@ -833,9 +821,6 @@ func (val *metricset) validate() error { return errors.Wrapf(err, "y") } for k, v := range val.Tags { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'g': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } switch t := v.(type) { case nil: case string: @@ -1063,14 +1048,6 @@ func (val *transactionMarks) validate() error { if !val.IsSet() { return nil } - for k, v := range val.Events { - if err := v.validate(); err != nil { - return errors.Wrapf(err, "events") - } - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'events': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } - } return nil } @@ -1088,11 +1065,6 @@ func (val *transactionMarkEvents) validate() error { if !val.IsSet() { return nil } - for k := range val.Measurements { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'measurements': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } - } return nil } @@ -1225,9 +1197,6 @@ func (val *spanContext) validate() error { return errors.Wrapf(err, "se") } for k, v := range val.Tags { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'g': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } switch t := v.(type) { case nil: case string: diff --git a/model/modeldecoder/rumv3/model_test.go b/model/modeldecoder/rumv3/model_test.go index c4dd98a2a79..47818944491 100644 --- a/model/modeldecoder/rumv3/model_test.go +++ b/model/modeldecoder/rumv3/model_test.go @@ -62,12 +62,9 @@ func TestServiceValidationRules(t *testing.T) { func TestLabelValidationRules(t *testing.T) { testcases := []testcase{ - {name: "valid", data: `{"k\\1":"v1\\.s*\"","k2":2.3,"k3":3,"k4":true,"k5":null}`}, + {name: "valid", data: `{"k.*\"\\1":"v1\\.s*\"","k2":2.3,"k3":3,"k4":true,"k5":null}`}, {name: "restricted-type", errorKey: "inputTypesVals", data: `{"k1":{"k2":"v1"}}`}, {name: "restricted-type", errorKey: "inputTypesVals", data: `{"k1":{"k2":[1,2,3]}}`}, - {name: "key-dot", errorKey: "patternKeys", data: `{"k.1":"v1"}`}, - {name: "key-asterisk", errorKey: "patternKeys", data: `{"k*1":"v1"}`}, - {name: "key-quotemark", errorKey: "patternKeys", data: `{"k\"1":"v1"}`}, {name: "max-len", data: `{"k1":"` + modeldecodertest.BuildString(1024) + `"}`}, {name: "max-len-exceeded", errorKey: "maxLengthVals", data: `{"k1":"` + modeldecodertest.BuildString(1025) + `"}`}, } @@ -90,10 +87,7 @@ func TestMaxLenValidationRules(t *testing.T) { func TestContextValidationRules(t *testing.T) { t.Run("custom", func(t *testing.T) { testcases := []testcase{ - {name: "custom", data: `{"cu":{"k1":{"v1":123,"v2":"value"},"k2":34,"k3":[{"a.1":1,"b*\"":2}]}}`}, - {name: "custom-key-dot", errorKey: "patternKeys", data: `{"cu":{"k1.":{"v1":123,"v2":"value"}}}`}, - {name: "custom-key-asterisk", errorKey: "patternKeys", data: `{"cu":{"k1*":{"v1":123,"v2":"value"}}}`}, - {name: "custom-key-quote", errorKey: "patternKeys", data: `{"cu":{"k1\"":{"v1":123,"v2":"value"}}}`}, + {name: "custom", data: `{"cu":{"k.*\"1":{"v1":123,"v2":"value"},"k2":34,"k3":[{"a.1":1,"b*\"":2}]}}`}, } testValidation(t, "x", testcases, "c") testValidation(t, "e", testcases, "c") @@ -110,13 +104,7 @@ func TestDurationValidationRules(t *testing.T) { func TestMarksValidationRules(t *testing.T) { testcases := []testcase{ - {name: "marks", data: `{"k1":{"v1":12.3}}`}, - {name: "marks-dot", errorKey: "patternKeys", data: `{"k.1":{"v1":12.3}}`}, - {name: "marks-event-dot", errorKey: "patternKeys", data: `{"k1":{"v.1":12.3}}`}, - {name: "marks-asterisk", errorKey: "patternKeys", data: `{"k*1":{"v1":12.3}}`}, - {name: "marks-event-asterisk", errorKey: "patternKeys", data: `{"k1":{"v*1":12.3}}`}, - {name: "marks-quote", errorKey: "patternKeys", data: `{"k\"1":{"v1":12.3}}`}, - {name: "marks-event-quote", errorKey: "patternKeys", data: `{"k1":{"v\"1":12.3}}`}, + {name: "marks", data: `{"k.*\"1":{"v.*\"1":12.3}}`}, } testValidation(t, "x", testcases, "k") } diff --git a/model/modeldecoder/rumv3/transaction_test.go b/model/modeldecoder/rumv3/transaction_test.go index 0a60a0a3bc9..97c4508ec91 100644 --- a/model/modeldecoder/rumv3/transaction_test.go +++ b/model/modeldecoder/rumv3/transaction_test.go @@ -154,8 +154,8 @@ func TestDecodeMapToTransactionModel(t *testing.T) { // metadata labels and transaction labels should not be merged mLabels := common.MapStr{"init0": "init", "init1": "init", "init2": "init"} assert.Equal(t, mLabels, out.Metadata.Labels) - tLabels := model.Labels{"overwritten0": "overwritten", "overwritten1": "overwritten"} - assert.Equal(t, &tLabels, out.Labels) + tLabels := common.MapStr{"overwritten0": "overwritten", "overwritten1": "overwritten"} + assert.Equal(t, tLabels, out.Labels) // service values should be set modeldecodertest.AssertStructValues(t, &out.Metadata.Service, metadataExceptions("Node", "Agent.EphemeralID"), otherVal) // user values should be set diff --git a/model/modeldecoder/v2/decoder.go b/model/modeldecoder/v2/decoder.go index 59d764681c8..96f860b6ca6 100644 --- a/model/modeldecoder/v2/decoder.go +++ b/model/modeldecoder/v2/decoder.go @@ -262,8 +262,7 @@ func mapToErrorModel(from *errorEvent, metadata *model.Metadata, reqTime time.Ti } // metadata labels and context labels are merged only in the output model if len(from.Context.Tags) > 0 { - labels := model.Labels(from.Context.Tags.Clone()) - out.Labels = &labels + out.Labels = from.Context.Tags.Clone() } if from.Context.Page.IsSet() { out.Page = &model.Page{} @@ -289,8 +288,7 @@ func mapToErrorModel(from *errorEvent, metadata *model.Metadata, reqTime time.Ti mapToRequestURLModel(from.Context.Request.URL, out.URL) } if len(from.Context.Custom) > 0 { - custom := model.Custom(from.Context.Custom.Clone()) - out.Custom = &custom + out.Custom = from.Context.Custom.Clone() } } if from.Culprit.IsSet() { @@ -1016,16 +1014,14 @@ func mapToTransactionModel(from *transaction, metadata *model.Metadata, reqTime if from.Context.IsSet() { if len(from.Context.Custom) > 0 { - custom := model.Custom(from.Context.Custom.Clone()) - out.Custom = &custom + out.Custom = from.Context.Custom.Clone() } if config.Experimental && from.Context.Experimental.IsSet() { out.Experimental = from.Context.Experimental.Val } // metadata labels and context labels are merged when transforming the output model if len(from.Context.Tags) > 0 { - labels := model.Labels(from.Context.Tags.Clone()) - out.Labels = &labels + out.Labels = from.Context.Tags.Clone() } if from.Context.Message.IsSet() { out.Message = &model.Message{} diff --git a/model/modeldecoder/v2/error_test.go b/model/modeldecoder/v2/error_test.go index 27cea6af69f..bb15aeca4f9 100644 --- a/model/modeldecoder/v2/error_test.go +++ b/model/modeldecoder/v2/error_test.go @@ -110,9 +110,9 @@ func TestDecodeMapToErrorModel(t *testing.T) { assert.Equal(t, ip, out.Metadata.Client.IP, out.Metadata.Client.IP.String()) // metadata labels and event labels should not be merged mLabels := common.MapStr{"init0": "init", "init1": "init", "init2": "init"} - tLabels := model.Labels{"overwritten0": "overwritten", "overwritten1": "overwritten"} + tLabels := common.MapStr{"overwritten0": "overwritten", "overwritten1": "overwritten"} assert.Equal(t, mLabels, out.Metadata.Labels) - assert.Equal(t, &tLabels, out.Labels) + assert.Equal(t, tLabels, out.Labels) // service and user values should be set modeldecodertest.AssertStructValues(t, &out.Metadata.Service, exceptions, otherVal) modeldecodertest.AssertStructValues(t, &out.Metadata.User, exceptions, otherVal) diff --git a/model/modeldecoder/v2/model.go b/model/modeldecoder/v2/model.go index 1f77efe62fe..ba91432c315 100644 --- a/model/modeldecoder/v2/model.go +++ b/model/modeldecoder/v2/model.go @@ -26,9 +26,8 @@ import ( ) var ( - patternAlphaNumericExt = `^[a-zA-Z0-9 _-]+$` - patternNoDotAsteriskQuote = `^[^.*"]*$` //do not allow '.' '*' '"' - patternNoAsteriskQuote = `^[^*"]*$` //do not allow '*' '"' + patternAlphaNumericExt = `^[a-zA-Z0-9 _-]+$` + patternNoAsteriskQuote = `^[^*"]*$` //do not allow '*' '"' enumOutcome = []string{"success", "failure", "unknown"} ) @@ -66,7 +65,7 @@ type context struct { // Custom can contain additional metadata to be stored with the event. // The format is unspecified and can be deeply nested objects. // The information will not be indexed or searchable in Elasticsearch. - Custom common.MapStr `json:"custom" validate:"patternKeys=patternNoDotAsteriskQuote"` + Custom common.MapStr `json:"custom"` // Experimental information is only processed when APM Server is started // in development mode and should only be used by APM agent developers // implementing new, unreleased features. The format is unspecified. @@ -89,7 +88,7 @@ type context struct { Service contextService `json:"service"` // Tags are a flat mapping of user-defined tags. Allowed value types are // string, boolean and number values. Tags are indexed and searchable. - Tags common.MapStr `json:"tags" validate:"patternKeys=patternNoDotAsteriskQuote,inputTypesVals=string;bool;number,maxLengthVals=1024"` + Tags common.MapStr `json:"tags" validate:"inputTypesVals=string;bool;number,maxLengthVals=1024"` // User holds information about the correlated user for this event. If // user data are provided here, all user related information from metadata // is ignored, otherwise the metadata's user information will be stored @@ -347,7 +346,7 @@ type metadata struct { Cloud metadataCloud `json:"cloud"` // Labels are a flat mapping of user-defined tags. Allowed value types are // string, boolean and number values. Labels are indexed and searchable. - Labels common.MapStr `json:"labels" validate:"patternKeys=patternNoDotAsteriskQuote,inputTypesVals=string;bool;number,maxLengthVals=1024"` + Labels common.MapStr `json:"labels" validate:"inputTypesVals=string;bool;number,maxLengthVals=1024"` // Process metadata about the monitored service. Process metadataProcess `json:"process"` // Service metadata about the monitored service. @@ -531,7 +530,7 @@ type metricset struct { Span metricsetSpanRef `json:"span"` // Tags are a flat mapping of user-defined tags. Allowed value types are // string, boolean and number values. Tags are indexed and searchable. - Tags common.MapStr `json:"tags" validate:"patternKeys=patternNoDotAsteriskQuote,inputTypesVals=string;bool;number,maxLengthVals=1024"` + Tags common.MapStr `json:"tags" validate:"inputTypesVals=string;bool;number,maxLengthVals=1024"` // Transaction holds selected information about the correlated transaction. Transaction metricsetTransactionRef `json:"transaction"` } @@ -625,7 +624,7 @@ type spanContext struct { Service contextService `json:"service"` // Tags are a flat mapping of user-defined tags. Allowed value types are // string, boolean and number values. Tags are indexed and searchable. - Tags common.MapStr `json:"tags" validate:"patternKeys=patternNoDotAsteriskQuote,inputTypesVals=string;bool;number,maxLengthVals=1024"` + Tags common.MapStr `json:"tags" validate:"inputTypesVals=string;bool;number,maxLengthVals=1024"` } type spanContextDatabase struct { @@ -774,7 +773,7 @@ type transaction struct { } type transactionMarks struct { - Events map[string]transactionMarkEvents `json:"-" validate:"patternKeys=patternNoDotAsteriskQuote"` + Events map[string]transactionMarkEvents `json:"-"` } func (m *transactionMarks) UnmarshalJSON(data []byte) error { @@ -782,7 +781,7 @@ func (m *transactionMarks) UnmarshalJSON(data []byte) error { } type transactionMarkEvents struct { - Measurements map[string]float64 `json:"-" validate:"patternKeys=patternNoDotAsteriskQuote"` + Measurements map[string]float64 `json:"-"` } func (m *transactionMarkEvents) UnmarshalJSON(data []byte) error { diff --git a/model/modeldecoder/v2/model_generated.go b/model/modeldecoder/v2/model_generated.go index 2437ca1f69a..50ee0924ddb 100644 --- a/model/modeldecoder/v2/model_generated.go +++ b/model/modeldecoder/v2/model_generated.go @@ -30,9 +30,8 @@ import ( ) var ( - patternAlphaNumericExtRegexp = regexp.MustCompile(patternAlphaNumericExt) - patternNoAsteriskQuoteRegexp = regexp.MustCompile(patternNoAsteriskQuote) - patternNoDotAsteriskQuoteRegexp = regexp.MustCompile(patternNoDotAsteriskQuote) + patternAlphaNumericExtRegexp = regexp.MustCompile(patternAlphaNumericExt) + patternNoAsteriskQuoteRegexp = regexp.MustCompile(patternNoAsteriskQuote) ) func (val *metadataRoot) IsSet() bool { @@ -76,9 +75,6 @@ func (val *metadata) validate() error { return errors.Wrapf(err, "cloud") } for k, v := range val.Labels { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'labels': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } switch t := v.(type) { case nil: case string: @@ -726,11 +722,6 @@ func (val *context) validate() error { if !val.IsSet() { return nil } - for k := range val.Custom { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'custom': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } - } if err := val.Message.validate(); err != nil { return errors.Wrapf(err, "message") } @@ -747,9 +738,6 @@ func (val *context) validate() error { return errors.Wrapf(err, "service") } for k, v := range val.Tags { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'tags': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } switch t := v.(type) { case nil: case string: @@ -1350,9 +1338,6 @@ func (val *metricset) validate() error { return errors.Wrapf(err, "span") } for k, v := range val.Tags { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'tags': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } switch t := v.(type) { case nil: case string: @@ -1594,9 +1579,6 @@ func (val *spanContext) validate() error { return errors.Wrapf(err, "service") } for k, v := range val.Tags { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'tags': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } switch t := v.(type) { case nil: case string: @@ -1857,14 +1839,6 @@ func (val *transactionMarks) validate() error { if !val.IsSet() { return nil } - for k, v := range val.Events { - if err := v.validate(); err != nil { - return errors.Wrapf(err, "events") - } - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'events': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } - } return nil } @@ -1882,11 +1856,6 @@ func (val *transactionMarkEvents) validate() error { if !val.IsSet() { return nil } - for k := range val.Measurements { - if k != "" && !patternNoDotAsteriskQuoteRegexp.MatchString(k) { - return fmt.Errorf("'measurements': validation rule 'patternKeys(patternNoDotAsteriskQuote)' violated") - } - } return nil } diff --git a/model/modeldecoder/v2/model_test.go b/model/modeldecoder/v2/model_test.go index c6f2defdd4c..594838bb084 100644 --- a/model/modeldecoder/v2/model_test.go +++ b/model/modeldecoder/v2/model_test.go @@ -70,12 +70,9 @@ func TestServiceValidationRules(t *testing.T) { func TestLabelValidationRules(t *testing.T) { testcases := []testcase{ - {name: "valid", data: `{"k1":"v1.s*\"","k2":2.3,"k3":3,"k4":true,"k5":null}`}, + {name: "valid", data: `{"k1\".*":"v1.s*\"","k2":2.3,"k3":3,"k4":true,"k5":null}`}, {name: "restricted-type", errorKey: "inputTypesVals", data: `{"k1":{"k2":"v1"}}`}, {name: "restricted-type-slice", errorKey: "inputTypesVals", data: `{"k1":{"v1":[1,2,3]}}`}, - {name: "key-dot", errorKey: "patternNoDotAsteriskQuote", data: `{"k.1":"v1"}`}, - {name: "key-asterisk", errorKey: "patternNoDotAsteriskQuote", data: `{"k*1":"v1"}`}, - {name: "key-quotemark", errorKey: "patternNoDotAsteriskQuote", data: `{"k\"1":"v1"}`}, {name: "max-len", data: `{"k1":"` + modeldecodertest.BuildString(1024) + `"}`}, {name: "max-len-exceeded", errorKey: "maxLengthVals", data: `{"k1":"` + modeldecodertest.BuildString(1025) + `"}`}, } @@ -107,10 +104,7 @@ func TestMaxLenValidationRules(t *testing.T) { func TestContextValidationRules(t *testing.T) { t.Run("custom", func(t *testing.T) { testcases := []testcase{ - {name: "custom", data: `{"custom":{"k\\1":{"v1":123,"v2":"value\\"},"k2":34,"k3":[{"a.1":1,"b*\"":2}]}}`}, - {name: "custom-key-dot", errorKey: "patternNoDotAsteriskQuote", data: `{"custom":{"k1.":{"v1":123,"v2":"value"}}}`}, - {name: "custom-key-asterisk", errorKey: "patternNoDotAsteriskQuote", data: `{"custom":{"k1*":{"v1":123,"v2":"value"}}}`}, - {name: "custom-key-quote", errorKey: "patternNoDotAsteriskQuote", data: `{"custom":{"k1\"":{"v1":123,"v2":"value"}}}`}, + {name: "custom", data: `{"custom":{"k\\1":{"v.1":123,"v*2":"value\\"},"k\"2":34,"k3":[{"a.1":1,"b*\"":2}]}}`}, } testValidation(t, "transaction", testcases, "context") }) @@ -136,13 +130,7 @@ func TestDurationValidationRules(t *testing.T) { func TestMarksValidationRules(t *testing.T) { testcases := []testcase{ - {name: "marks", data: `{"k1\\":{"v1\\":12.3}}`}, - {name: "marks-dot", errorKey: "patternNoDotAsteriskQuote", data: `{"k.1":{"v1":12.3}}`}, - {name: "marks-events-dot", errorKey: "patternNoDotAsteriskQuote", data: `{"k1":{"v.1":12.3}}`}, - {name: "marks-asterisk", errorKey: "patternNoDotAsteriskQuote", data: `{"k*1":{"v1":12.3}}`}, - {name: "marks-events-asterisk", errorKey: "patternNoDotAsteriskQuote", data: `{"k1":{"v*1":12.3}}`}, - {name: "marks-quote", errorKey: "patternNoDotAsteriskQuote", data: `{"k\"1":{"v1":12.3}}`}, - {name: "marks-events-quote", errorKey: "patternNoDotAsteriskQuote", data: `{"k1":{"v\"1":12.3}}`}, + {name: "marks", data: `{"k.1*\\\"":{"v.1*\\\"":12.3}}`}, } testValidation(t, "transaction", testcases, "marks") } diff --git a/model/modeldecoder/v2/transaction_test.go b/model/modeldecoder/v2/transaction_test.go index b31bfacb40b..6e728b35f26 100644 --- a/model/modeldecoder/v2/transaction_test.go +++ b/model/modeldecoder/v2/transaction_test.go @@ -112,9 +112,9 @@ func TestDecodeMapToTransactionModel(t *testing.T) { assert.Equal(t, ip, out.Metadata.Client.IP, out.Metadata.Client.IP.String()) // metadata labels and event labels should not be merged mLabels := common.MapStr{"init0": "init", "init1": "init", "init2": "init"} - tLabels := model.Labels{"overwritten0": "overwritten", "overwritten1": "overwritten"} + tLabels := common.MapStr{"overwritten0": "overwritten", "overwritten1": "overwritten"} assert.Equal(t, mLabels, out.Metadata.Labels) - assert.Equal(t, &tLabels, out.Labels) + assert.Equal(t, tLabels, out.Labels) // service and user values should be set modeldecodertest.AssertStructValues(t, &out.Metadata.Service, exceptions, otherVal) modeldecodertest.AssertStructValues(t, &out.Metadata.User, exceptions, otherVal) diff --git a/model/profile.go b/model/profile.go index e60ae24e6a5..4c6c3a9f494 100644 --- a/model/profile.go +++ b/model/profile.go @@ -133,14 +133,14 @@ func (pp PprofProfile) Transform(ctx context.Context, cfg *transform.Config) []b event.Fields[datastreams.TypeField] = datastreams.MetricsType event.Fields[datastreams.DatasetField] = dataset } - pp.Metadata.Set(event.Fields) + var profileLabels common.MapStr if len(sample.Label) > 0 { - labels := make(common.MapStr) + profileLabels = make(common.MapStr) for k, v := range sample.Label { - utility.Set(labels, k, v) + profileLabels[k] = v } - utility.DeepUpdate(event.Fields, "labels", labels) } + pp.Metadata.Set(event.Fields, profileLabels) samples[i] = event } return samples diff --git a/model/span.go b/model/span.go index 7c6ca7230e6..024e7e2f256 100644 --- a/model/span.go +++ b/model/span.go @@ -204,13 +204,11 @@ func (e *Span) Transform(ctx context.Context, cfg *transform.Config) []beat.Even } // first set the generic metadata - e.Metadata.Set(fields) + e.Metadata.Set(fields, e.Labels) // then add event specific information utility.DeepUpdate(fields, "service", e.Service.Fields("", "")) utility.DeepUpdate(fields, "agent", e.Service.AgentFields()) - // merges with metadata labels, overrides conflicting keys - utility.DeepUpdate(fields, "labels", e.Labels) utility.AddID(fields, "parent", e.ParentID) if e.ChildIDs != nil { utility.Set(fields, "child", common.MapStr{"id": e.ChildIDs}) diff --git a/model/span_test.go b/model/span_test.go index 3a803ca4bbf..64789fac59c 100644 --- a/model/span_test.go +++ b/model/span_test.go @@ -68,7 +68,7 @@ func TestSpanTransform(t *testing.T) { "type": "", }, "event": common.MapStr{"outcome": ""}, - "labels": metadataLabels, + "labels": common.MapStr{"label_a": "a", "label_b": "b", "c": 1}, "timestamp": common.MapStr{"us": timestampUs}, }, }, @@ -86,7 +86,7 @@ func TestSpanTransform(t *testing.T) { "type": "", }, "timestamp": common.MapStr{"us": timestampUs}, - "labels": metadataLabels, + "labels": common.MapStr{"label_a": "a", "label_b": "b", "c": 1}, "event": common.MapStr{"outcome": "success"}, }, }, @@ -108,7 +108,7 @@ func TestSpanTransform(t *testing.T) { Duration: 1.20, RUM: true, Stacktrace: Stacktrace{{AbsPath: &path}}, - Labels: common.MapStr{"label.a": 12}, + Labels: common.MapStr{"label_a": 12}, HTTP: &HTTP{Method: &method, StatusCode: &statusCode, URL: &url}, DB: &DB{ Instance: &instance, @@ -163,7 +163,7 @@ func TestSpanTransform(t *testing.T) { }, "message": common.MapStr{"queue": common.MapStr{"name": "users"}}, }, - "labels": common.MapStr{"label.a": 12, "label.b": "b", "c": 1}, + "labels": common.MapStr{"label_a": 12, "label_b": "b", "c": 1}, "processor": common.MapStr{"event": "span", "name": "transaction"}, "service": common.MapStr{"name": serviceName, "environment": env, "version": serviceVersion}, "timestamp": common.MapStr{"us": timestampUs}, diff --git a/model/system_test.go b/model/system_test.go index f47bb085635..d4abdff62e5 100644 --- a/model/system_test.go +++ b/model/system_test.go @@ -62,7 +62,7 @@ func TestSystemTransformation(t *testing.T) { t.Run(name, func(t *testing.T) { fields := make(common.MapStr) metadata := &Metadata{System: system} - metadata.Set(fields) + metadata.Set(fields, nil) resultJSON, err := json.Marshal(fields["host"]) require.NoError(t, err) name := filepath.Join("test_approved", "system", strings.ReplaceAll(name, " ", "_")) diff --git a/model/transaction.go b/model/transaction.go index 4d54076cc0b..3b4640642a6 100644 --- a/model/transaction.go +++ b/model/transaction.go @@ -63,8 +63,8 @@ type Transaction struct { Page *Page HTTP *Http URL *URL - Labels *Labels - Custom *Custom + Labels common.MapStr + Custom common.MapStr UserExperience *UserExperience Experimental interface{} @@ -91,7 +91,7 @@ func (e *Transaction) fields() common.MapStr { fields.maybeSetString("result", e.Result) fields.maybeSetMapStr("marks", e.Marks.fields()) fields.maybeSetMapStr("page", e.Page.Fields()) - fields.maybeSetMapStr("custom", e.Custom.Fields()) + fields.maybeSetMapStr("custom", customFields(e.Custom)) fields.maybeSetMapStr("message", e.Message.Fields()) fields.maybeSetMapStr("experience", e.UserExperience.Fields()) if e.SpanCount.Dropped != nil || e.SpanCount.Started != nil { @@ -126,15 +126,13 @@ func (e *Transaction) Transform(_ context.Context, cfg *transform.Config) []beat } // first set generic metadata (order is relevant) - e.Metadata.Set(fields) + e.Metadata.Set(fields, e.Labels) utility.Set(fields, "source", fields["client"]) // then merge event specific information utility.AddID(fields, "parent", e.ParentID) utility.AddID(fields, "trace", e.TraceID) utility.Set(fields, "timestamp", utility.TimeAsMicros(e.Timestamp)) - // merges with metadata labels, overrides conflicting keys - utility.DeepUpdate(fields, "labels", e.Labels.Fields()) utility.Set(fields, "http", e.HTTP.Fields()) urlFields := e.URL.Fields() if urlFields != nil { @@ -159,7 +157,7 @@ func (m TransactionMarks) fields() common.MapStr { } out := make(mapStr, len(m)) for k, v := range m { - out.maybeSetMapStr(k, v.fields()) + out.maybeSetMapStr(sanitizeLabelKey(k), v.fields()) } return common.MapStr(out) } @@ -172,7 +170,7 @@ func (m TransactionMark) fields() common.MapStr { } out := make(common.MapStr, len(m)) for k, v := range m { - out[k] = common.Float(v) + out[sanitizeLabelKey(k)] = common.Float(v) } return out } diff --git a/model/transaction_test.go b/model/transaction_test.go index 981b57b3bb5..cef19b0a8ec 100644 --- a/model/transaction_test.go +++ b/model/transaction_test.go @@ -171,11 +171,11 @@ func TestEventsTransformWithMetadata(t *testing.T) { txWithContext := Transaction{ Metadata: eventMetadata, Timestamp: timestamp, - Labels: &Labels{"a": "b"}, + Labels: common.MapStr{"a": "b"}, Page: &Page{URL: &URL{Original: &url}, Referer: &referer}, HTTP: &Http{Request: &request, Response: &response}, URL: &URL{Original: &url}, - Custom: &Custom{"foo": "bar"}, + Custom: common.MapStr{"foo.bar": "baz"}, Message: &Message{QueueName: tests.StringPtr("routeUser")}, } events := txWithContext.Transform(context.Background(), &transform.Config{DataStreams: true}) @@ -212,7 +212,7 @@ func TestEventsTransformWithMetadata(t *testing.T) { "sampled": true, "page": common.MapStr{"url": url, "referer": referer}, "custom": common.MapStr{ - "foo": "bar", + "foo_bar": "baz", }, "message": common.MapStr{"queue": common.MapStr{"name": "routeUser"}}, }, @@ -282,3 +282,33 @@ func TestTransactionTransformPage(t *testing.T) { assert.Equal(t, test.Output, output[0].Fields["url"], fmt.Sprintf("Failed at idx %v; %s", idx, test.Msg)) } } + +func TestTransactionTransformMarks(t *testing.T) { + tests := []struct { + Transaction Transaction + Output common.MapStr + Msg string + }{ + { + Transaction: Transaction{ + Marks: TransactionMarks{ + "a.b": TransactionMark{ + "c.d": 123, + }, + }, + }, + Output: common.MapStr{ + "a_b": common.MapStr{ + "c_d": common.Float(123), + }, + }, + Msg: "Unsanitized transaction mark names", + }, + } + + for idx, test := range tests { + output := test.Transaction.Transform(context.Background(), &transform.Config{}) + marks, _ := output[0].Fields.GetValue("transaction.marks") + assert.Equal(t, test.Output, marks, fmt.Sprintf("Failed at idx %v; %s", idx, test.Msg)) + } +} diff --git a/processor/otel/consumer.go b/processor/otel/consumer.go index b57e71f280f..a5e2932fd78 100644 --- a/processor/otel/consumer.go +++ b/processor/otel/consumer.go @@ -201,13 +201,13 @@ func parseMetadata(td consumerdata.TraceData, md *model.Metadata) { md.Labels = make(common.MapStr) for key, val := range td.Node.GetAttributes() { - md.Labels[replaceDots(key)] = truncate(val) + md.Labels[key] = truncate(val) } if t := td.Resource.GetType(); t != "" { md.Labels["resource"] = truncate(t) } for key, val := range td.Resource.GetLabels() { - md.Labels[replaceDots(key)] = truncate(val) + md.Labels[key] = truncate(val) } } @@ -236,19 +236,19 @@ func parseTransaction(span *tracepb.Span, sourceFormat string, hostname string, k := replaceDots(kDots) switch v := v.Value.(type) { case *tracepb.AttributeValue_BoolValue: - utility.DeepUpdate(labels, k, v.BoolValue) + labels[k] = v.BoolValue if k == "error" { hasFailed = v.BoolValue } case *tracepb.AttributeValue_DoubleValue: - utility.DeepUpdate(labels, k, v.DoubleValue) + labels[k] = v.DoubleValue case *tracepb.AttributeValue_IntValue: switch kDots { case "http.status_code": httpStatusCode = int(v.IntValue) isHTTP = true default: - utility.DeepUpdate(labels, k, v.IntValue) + labels[k] = v.IntValue } case *tracepb.AttributeValue_StringValue: switch kDots { @@ -269,7 +269,7 @@ func parseTransaction(span *tracepb.Span, sourceFormat string, hostname string, version := truncate(strings.TrimPrefix(v.StringValue.Value, "HTTP/")) http.Version = &version } else { - utility.DeepUpdate(labels, k, v.StringValue.Value) + labels[k] = v.StringValue.Value } isHTTP = true case "message_bus.destination": @@ -283,7 +283,7 @@ func parseTransaction(span *tracepb.Span, sourceFormat string, hostname string, component = truncate(v.StringValue.Value) fallthrough default: - utility.DeepUpdate(labels, k, truncate(v.StringValue.Value)) + labels[k] = truncate(v.StringValue.Value) } } } @@ -332,11 +332,7 @@ func parseTransaction(span *tracepb.Span, sourceFormat string, hostname string, parseSamplerAttributes(samplerType, samplerParam, &event.RepresentativeCount, labels) } - if len(labels) == 0 { - return - } - l := model.Labels(labels) - event.Labels = &l + event.Labels = labels } func parseSpan(span *tracepb.Span, sourceFormat string, event *model.Span) { @@ -365,9 +361,9 @@ func parseSpan(span *tracepb.Span, sourceFormat string, event *model.Span) { k := replaceDots(kDots) switch v := v.Value.(type) { case *tracepb.AttributeValue_BoolValue: - utility.DeepUpdate(labels, k, v.BoolValue) + labels[k] = v.BoolValue case *tracepb.AttributeValue_DoubleValue: - utility.DeepUpdate(labels, k, v.DoubleValue) + labels[k] = v.DoubleValue case *tracepb.AttributeValue_IntValue: switch kDots { case "http.status_code": @@ -378,7 +374,7 @@ func parseSpan(span *tracepb.Span, sourceFormat string, event *model.Span) { port := int(v.IntValue) destination.Port = &port default: - utility.DeepUpdate(labels, k, v.IntValue) + labels[k] = v.IntValue } case *tracepb.AttributeValue_StringValue: switch kDots { @@ -441,7 +437,7 @@ func parseSpan(span *tracepb.Span, sourceFormat string, event *model.Span) { component = truncate(v.StringValue.Value) fallthrough default: - utility.DeepUpdate(labels, k, truncate(v.StringValue.Value)) + labels[k] = truncate(v.StringValue.Value) } } } @@ -534,9 +530,6 @@ func parseSpan(span *tracepb.Span, sourceFormat string, event *model.Span) { parseSamplerAttributes(samplerType, samplerParam, &event.RepresentativeCount, labels) } - if len(labels) == 0 { - return - } event.Labels = labels } @@ -548,12 +541,12 @@ func parseSamplerAttributes(samplerType, samplerParam *tracepb.AttributeValue, r *representativeCount = 1 / probability } default: - utility.DeepUpdate(labels, "sampler_type", samplerType.GetStringValue().GetValue()) + labels["sampler_type"] = samplerType.GetStringValue().GetValue() switch v := samplerParam.Value.(type) { case *tracepb.AttributeValue_BoolValue: - utility.DeepUpdate(labels, "sampler_param", v.BoolValue) + labels["sampler_param"] = v.BoolValue case *tracepb.AttributeValue_DoubleValue: - utility.DeepUpdate(labels, "sampler_param", v.DoubleValue) + labels["sampler_param"] = v.DoubleValue } } }