From 0774e67901d9f353bbae5a5d7d07b12ff801fcd5 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Sat, 17 Jul 2021 17:42:03 +0800 Subject: [PATCH] Move model value normalisation to modeldecoder When modeldecoder decodes numbers and stores them in interface{} fields, it has been storing them as json.Numbers and we would convert these to numbers during model transformation. As this is specific to modeldecoder and not any other inputs (e.g. OTLP), move this "normalisation" logic to modeldecoder. --- model/context.go | 7 ++-- model/error.go | 25 +++--------- model/error_test.go | 17 +------- model/labels.go | 22 ++--------- .../modeldecoderutil/exception.go | 38 ++++++++++++++++++ model/modeldecoder/modeldecoderutil/labels.go | 39 +++++++++++++++++++ model/modeldecoder/rumv3/decoder.go | 19 ++++----- model/modeldecoder/rumv3/error_test.go | 8 ++++ model/modeldecoder/v2/decoder.go | 16 ++++---- model/modeldecoder/v2/error_test.go | 8 ++++ 10 files changed, 122 insertions(+), 77 deletions(-) create mode 100644 model/modeldecoder/modeldecoderutil/exception.go create mode 100644 model/modeldecoder/modeldecoderutil/labels.go diff --git a/model/context.go b/model/context.go index 8e814e8c9c8..711e6ff77b5 100644 --- a/model/context.go +++ b/model/context.go @@ -41,16 +41,15 @@ func (page *Page) Fields() common.MapStr { return common.MapStr(fields) } -// customFields transforms in, returning a copy with sanitized keys -// and normalized field values, suitable for storing as "custom" -// in transaction and error documents.. +// customFields transforms in, returning a copy with sanitized keys, +// suitable for storing as "custom" in transaction and error documents. func customFields(in common.MapStr) common.MapStr { if len(in) == 0 { return nil } out := make(common.MapStr, len(in)) for k, v := range in { - out[sanitizeLabelKey(k)] = normalizeLabelValue(v) + out[sanitizeLabelKey(k)] = v } return out } diff --git a/model/error.go b/model/error.go index c403746ca52..2b9df620c93 100644 --- a/model/error.go +++ b/model/error.go @@ -19,9 +19,6 @@ package model import ( "context" - "encoding/json" - "fmt" - "strconv" "time" "github.com/elastic/beats/v7/libbeat/beat" @@ -74,7 +71,7 @@ type Error struct { type Exception struct { Message string Module string - Code interface{} + Code string Attributes interface{} Stacktrace Stacktrace Type string @@ -160,12 +157,13 @@ func (e *Error) fields() common.MapStr { } func (e *Error) exceptionFields(chain []Exception) []common.MapStr { - var result []common.MapStr - for _, exception := range chain { + result := make([]common.MapStr, len(chain)) + for i, exception := range chain { var ex mapStr ex.maybeSetString("message", exception.Message) ex.maybeSetString("module", exception.Module) ex.maybeSetString("type", exception.Type) + ex.maybeSetString("code", exception.Code) ex.maybeSetBool("handled", exception.Handled) if exception.Parent != nil { ex.set("parent", exception.Parent) @@ -173,18 +171,6 @@ func (e *Error) exceptionFields(chain []Exception) []common.MapStr { if exception.Attributes != nil { ex.set("attributes", exception.Attributes) } - - switch code := exception.Code.(type) { - case int: - ex.set("code", strconv.Itoa(code)) - case float64: - ex.set("code", fmt.Sprintf("%.0f", code)) - case string: - ex.set("code", code) - case json.Number: - ex.set("code", code.String()) - } - if n := len(exception.Stacktrace); n > 0 { frames := make([]common.MapStr, n) for i, frame := range exception.Stacktrace { @@ -192,8 +178,7 @@ func (e *Error) exceptionFields(chain []Exception) []common.MapStr { } ex.set("stacktrace", frames) } - - result = append(result, common.MapStr(ex)) + result[i] = common.MapStr(ex) } return result } diff --git a/model/error_test.go b/model/error_test.go index a544ec97b80..8d98b5cb094 100644 --- a/model/error_test.go +++ b/model/error_test.go @@ -33,7 +33,7 @@ func baseException() *Exception { return &Exception{Message: "exception message"} } -func (e *Exception) withCode(code interface{}) *Exception { +func (e *Exception) withCode(code string) *Exception { e.Code = code return e } @@ -92,14 +92,13 @@ func TestEventFields(t *testing.T) { culprit := "some trigger" errorType := "error type" - codeFloat := 13.0 module := "error module" exMsg := "exception message" handled := false attributes := common.MapStr{"k1": "val1"} exception := Exception{ Type: errorType, - Code: codeFloat, + Code: "13", Message: exMsg, Module: module, Handled: &handled, @@ -157,18 +156,6 @@ func TestEventFields(t *testing.T) { "exception": []common.MapStr{{"message": "exception message", "code": "13"}}, }, }, - "intCode": { - Error: Error{Exception: baseException().withCode(13)}, - Output: common.MapStr{ - "exception": []common.MapStr{{"message": "exception message", "code": "13"}}, - }, - }, - "floatCode": { - Error: Error{Exception: baseException().withCode(13.0)}, - Output: common.MapStr{ - "exception": []common.MapStr{{"message": "exception message", "code": "13"}}, - }, - }, "withFrames": { Error: Error{ ID: id, diff --git a/model/labels.go b/model/labels.go index de8c4dfa308..66c1feca074 100644 --- a/model/labels.go +++ b/model/labels.go @@ -18,7 +18,6 @@ package model import ( - "encoding/json" "strings" "github.com/elastic/beats/v7/libbeat/common" @@ -40,32 +39,17 @@ func maybeSetLabels(out *mapStr, globalLabels, eventLabels common.MapStr) { if v == nil { continue } - k := sanitizeLabelKey(k) - combined[k] = normalizeLabelValue(v) + combined[sanitizeLabelKey(k)] = v } for k, v := range eventLabels { - k := sanitizeLabelKey(k) if v == nil { - delete(combined, k) - } else { - combined[k] = normalizeLabelValue(v) + continue } + combined[sanitizeLabelKey(k)] = 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) } diff --git a/model/modeldecoder/modeldecoderutil/exception.go b/model/modeldecoder/modeldecoderutil/exception.go new file mode 100644 index 00000000000..ccc6e24ba24 --- /dev/null +++ b/model/modeldecoder/modeldecoderutil/exception.go @@ -0,0 +1,38 @@ +// 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 modeldecoderutil + +import ( + "encoding/json" + "strconv" +) + +// ExceptionCodeString formats the exception code v as a string. +func ExceptionCodeString(v interface{}) string { + switch v := v.(type) { + case int: + return strconv.Itoa(v) + case float64: + return strconv.Itoa(int(v)) + case string: + return v + case json.Number: + return v.String() + } + return "" +} diff --git a/model/modeldecoder/modeldecoderutil/labels.go b/model/modeldecoder/modeldecoderutil/labels.go new file mode 100644 index 00000000000..308ab24ef5f --- /dev/null +++ b/model/modeldecoder/modeldecoderutil/labels.go @@ -0,0 +1,39 @@ +// 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 modeldecoderutil + +import ( + "encoding/json" + + "github.com/elastic/beats/v7/libbeat/common" +) + +// NormalizeLabelValues transforms the values in labels, replacing any +// instance of json.Number with libbeat/common.Float, and returning +// labels. +func NormalizeLabelValues(labels common.MapStr) common.MapStr { + for k, v := range labels { + switch v := v.(type) { + case json.Number: + if floatVal, err := v.Float64(); err == nil { + labels[k] = common.Float(floatVal) + } + } + } + return labels +} diff --git a/model/modeldecoder/rumv3/decoder.go b/model/modeldecoder/rumv3/decoder.go index 38f0a506bd7..de21ef0bbfe 100644 --- a/model/modeldecoder/rumv3/decoder.go +++ b/model/modeldecoder/rumv3/decoder.go @@ -26,8 +26,6 @@ import ( "sync" "time" - "github.com/elastic/beats/v7/libbeat/common" - "github.com/elastic/apm-server/decoder" "github.com/elastic/apm-server/model" "github.com/elastic/apm-server/model/modeldecoder" @@ -209,7 +207,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 { - out.Labels = from.Context.Tags.Clone() + out.Labels = modeldecoderutil.NormalizeLabelValues(from.Context.Tags.Clone()) } if from.Context.Request.IsSet() { out.HTTP = &model.HTTP{Request: &model.HTTPRequest{}} @@ -240,7 +238,7 @@ func mapToErrorModel(from *errorEvent, metadata *model.Metadata, reqTime time.Ti } } if len(from.Context.Custom) > 0 { - out.Custom = from.Context.Custom.Clone() + out.Custom = modeldecoderutil.NormalizeLabelValues(from.Context.Custom.Clone()) } } if from.Culprit.IsSet() { @@ -307,7 +305,7 @@ func mapToExceptionModel(from errorException, out *model.Exception) { out.Attributes = from.Attributes.Clone() } if from.Code.IsSet() { - out.Code = from.Code.Val + out.Code = modeldecoderutil.ExceptionCodeString(from.Code.Val) } if len(from.Cause) > 0 { out.Cause = make([]model.Exception, len(from.Cause)) @@ -338,8 +336,7 @@ func mapToExceptionModel(from errorException, out *model.Exception) { func mapToMetadataModel(m *metadata, out *model.Metadata) { // Labels if len(m.Labels) > 0 { - out.Labels = common.MapStr{} - out.Labels.Update(m.Labels) + out.Labels = modeldecoderutil.NormalizeLabelValues(m.Labels.Clone()) } // Service @@ -434,7 +431,7 @@ func mapToMetricsetModel(from *metricset, metadata *model.Metadata, reqTime time } if len(from.Tags) > 0 { - out.Labels = from.Tags.Clone() + out.Labels = modeldecoderutil.NormalizeLabelValues(from.Tags.Clone()) } // map span information if from.Span.Subtype.IsSet() { @@ -619,7 +616,7 @@ func mapToSpanModel(from *span, metadata *model.Metadata, reqTime time.Time, out } } if len(from.Context.Tags) > 0 { - out.Labels = from.Context.Tags.Clone() + out.Labels = modeldecoderutil.NormalizeLabelValues(from.Context.Tags.Clone()) } if from.Duration.IsSet() { out.Duration = from.Duration.Val @@ -724,11 +721,11 @@ func mapToTransactionModel(from *transaction, metadata *model.Metadata, reqTime if from.Context.IsSet() { if len(from.Context.Custom) > 0 { - out.Custom = from.Context.Custom.Clone() + out.Custom = modeldecoderutil.NormalizeLabelValues(from.Context.Custom.Clone()) } // metadata labels and context labels are merged when transforming the output model if len(from.Context.Tags) > 0 { - out.Labels = from.Context.Tags.Clone() + out.Labels = modeldecoderutil.NormalizeLabelValues(from.Context.Tags.Clone()) } if from.Context.Request.IsSet() { out.HTTP = &model.HTTP{Request: &model.HTTPRequest{}} diff --git a/model/modeldecoder/rumv3/error_test.go b/model/modeldecoder/rumv3/error_test.go index e6546e18552..e2856809937 100644 --- a/model/modeldecoder/rumv3/error_test.go +++ b/model/modeldecoder/rumv3/error_test.go @@ -212,4 +212,12 @@ func TestDecodeMapToErrorModel(t *testing.T) { assert.Equal(t, common.MapStr{"a": []string{"b"}, "c": []string{"d", "e"}}, out.HTTP.Request.Headers) assert.Equal(t, common.MapStr{"f": []string{"g"}}, out.HTTP.Response.Headers) }) + + t.Run("exception-code", func(t *testing.T) { + var input errorEvent + var out model.Error + input.Exception.Code.Set(123.456) + mapToErrorModel(&input, &model.Metadata{}, time.Now(), &out) + assert.Equal(t, "123", out.Exception.Code) + }) } diff --git a/model/modeldecoder/v2/decoder.go b/model/modeldecoder/v2/decoder.go index 94cff12f1b0..eebafca5973 100644 --- a/model/modeldecoder/v2/decoder.go +++ b/model/modeldecoder/v2/decoder.go @@ -263,7 +263,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 { - out.Labels = from.Context.Tags.Clone() + out.Labels = modeldecoderutil.NormalizeLabelValues(from.Context.Tags.Clone()) } if from.Context.Request.IsSet() { out.HTTP = &model.HTTP{Request: &model.HTTPRequest{}} @@ -302,7 +302,7 @@ func mapToErrorModel(from *errorEvent, metadata *model.Metadata, reqTime time.Ti } } if len(from.Context.Custom) > 0 { - out.Custom = from.Context.Custom.Clone() + out.Custom = modeldecoderutil.NormalizeLabelValues(from.Context.Custom.Clone()) } } if from.Culprit.IsSet() { @@ -366,7 +366,7 @@ func mapToExceptionModel(from errorException, out *model.Exception) { out.Attributes = from.Attributes.Clone() } if from.Code.IsSet() { - out.Code = from.Code.Val + out.Code = modeldecoderutil.ExceptionCodeString(from.Code.Val) } if len(from.Cause) > 0 { out.Cause = make([]model.Exception, len(from.Cause)) @@ -435,7 +435,7 @@ func mapToMetadataModel(from *metadata, out *model.Metadata) { // Labels if len(from.Labels) > 0 { - out.Labels = from.Labels.Clone() + out.Labels = modeldecoderutil.NormalizeLabelValues(from.Labels.Clone()) } // Process @@ -583,7 +583,7 @@ func mapToMetricsetModel(from *metricset, metadata *model.Metadata, reqTime time } if len(from.Tags) > 0 { - out.Labels = from.Tags.Clone() + out.Labels = modeldecoderutil.NormalizeLabelValues(from.Tags.Clone()) } // map span information if from.Span.Subtype.IsSet() { @@ -896,7 +896,7 @@ func mapToSpanModel(from *span, metadata *model.Metadata, reqTime time.Time, con mapToServiceModel(from.Context.Service, &out.Metadata.Service) } if len(from.Context.Tags) > 0 { - out.Labels = from.Context.Tags.Clone() + out.Labels = modeldecoderutil.NormalizeLabelValues(from.Context.Tags.Clone()) } if from.Duration.IsSet() { out.Duration = from.Duration.Val @@ -1023,14 +1023,14 @@ func mapToTransactionModel(from *transaction, metadata *model.Metadata, reqTime if from.Context.IsSet() { if len(from.Context.Custom) > 0 { - out.Custom = from.Context.Custom.Clone() + out.Custom = modeldecoderutil.NormalizeLabelValues(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 { - out.Labels = from.Context.Tags.Clone() + out.Labels = modeldecoderutil.NormalizeLabelValues(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 8b8ca5f92bb..9a06e85d36a 100644 --- a/model/modeldecoder/v2/error_test.go +++ b/model/modeldecoder/v2/error_test.go @@ -219,4 +219,12 @@ func TestDecodeMapToErrorModel(t *testing.T) { assert.Equal(t, "https://my.site.test:9201", out.Page.Referer) assert.Equal(t, "https://my.site.test:9201", out.HTTP.Request.Referrer) }) + + t.Run("exception-code", func(t *testing.T) { + var input errorEvent + var out model.Error + input.Exception.Code.Set(123.456) + mapToErrorModel(&input, &model.Metadata{}, time.Now(), modeldecoder.Config{}, &out) + assert.Equal(t, "123", out.Exception.Code) + }) }