Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move model value normalisation to modeldecoder #5784

Merged
merged 1 commit into from
Jul 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions model/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
25 changes: 5 additions & 20 deletions model/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ package model

import (
"context"
"encoding/json"
"fmt"
"strconv"
"time"

"github.com/elastic/beats/v7/libbeat/beat"
Expand Down Expand Up @@ -74,7 +71,7 @@ type Error struct {
type Exception struct {
Message string
Module string
Code interface{}
Code string
Attributes interface{}
Stacktrace Stacktrace
Type string
Expand Down Expand Up @@ -160,40 +157,28 @@ 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)
}
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 {
frames[i] = frame.transform()
}
ex.set("stacktrace", frames)
}

result = append(result, common.MapStr(ex))
result[i] = common.MapStr(ex)
}
return result
}
Expand Down
17 changes: 2 additions & 15 deletions model/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 3 additions & 19 deletions model/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package model

import (
"encoding/json"
"strings"

"github.com/elastic/beats/v7/libbeat/common"
Expand All @@ -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)
}
Expand Down
38 changes: 38 additions & 0 deletions model/modeldecoder/modeldecoderutil/exception.go
Original file line number Diff line number Diff line change
@@ -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 ""
}
39 changes: 39 additions & 0 deletions model/modeldecoder/modeldecoderutil/labels.go
Original file line number Diff line number Diff line change
@@ -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
}
19 changes: 8 additions & 11 deletions model/modeldecoder/rumv3/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{}}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{}}
Expand Down
8 changes: 8 additions & 0 deletions model/modeldecoder/rumv3/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Loading