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

Implement length limits for CLM attributes #602

Merged
merged 2 commits into from
Nov 15, 2022
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
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
## Unreleased (working notes)

### Changed

* Changed the following `TraceOption` function to be consistent with their usage and other related identifier names. The old names remain for backward compatibility, but new code should use the new names.
* `WithIgnoredPrefix` -> `WithIgnoredPrefixes`
* `WithPathPrefix` -> `WithPathPrefixes`
* Implemented better handling of Code Level Metrics reporting when the data (e.g., function names) are excessively long, so that those attributes are suppressed rather than being reported with truncated names. Specifically:
* Attributes with values longer than 255 characters are dropped.
* No CLM attributes at all will be attached to a trace if the `code.function` attribute is empty or is longer than 255 characters.
* No CLM attributes at all will be attached to a trace if both `code.namespace` and `code.filepath` are longer than 255 characters.

## 3.20.0

**PLEASE READ** these changes, and verify your config settings to ensure your application behaves how you intend it to. This release changes some default behaviors in the go agent.
Expand Down
18 changes: 14 additions & 4 deletions v3/newrelic/code_level_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,8 +612,18 @@ func reportCodeLevelMetrics(tOpts traceOptSet, run *appRun, setAttr func(string,
function = location.Function[ns+1:]
}

setAttr(AttributeCodeLineno, "", location.LineNo)
setAttr(AttributeCodeNamespace, namespace, nil)
setAttr(AttributeCodeFilepath, location.FilePath, nil)
setAttr(AttributeCodeFunction, function, nil)
// Impose data value size limits.
// Report no field over 255 characters in length.
// Report no CLM data at all if the function name is empty or >255 chars.
// Report no CLM data at all if both namespace and file path are >255 chars.
if function != "" && len(function) <= 255 && (len(namespace) <= 255 || len(location.FilePath) <= 255) {
setAttr(AttributeCodeLineno, "", location.LineNo)
setAttr(AttributeCodeFunction, function, nil)
if len(namespace) <= 255 {
setAttr(AttributeCodeNamespace, namespace, nil)
}
if len(location.FilePath) <= 255 {
setAttr(AttributeCodeFilepath, location.FilePath, nil)
}
}
}
105 changes: 104 additions & 1 deletion v3/newrelic/code_level_metrics_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Copyright 2020 New Relic Corporation. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

package newrelic

import (
"fmt"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -253,3 +253,106 @@ func skipCCached(t *testing.T) {
func TestCLMSkipCached(t *testing.T) {
skipACached(t)
}

func attributeMapMatchesCLM(expected, actual map[string]interface{}) error {
for k, v := range expected {
actualValue, present := actual[k]
if !present {
return fmt.Errorf("Expected field \"%s\" was not present in output", k)
}

switch value := v.(type) {
case int:
act, ok := actualValue.(int)
if !ok {
return fmt.Errorf("Expected value %v for %s was actually %v of type %T, not int",
v, k, actualValue, actualValue)
}

if act != value {
return fmt.Errorf("Expected %s value %v but got %v", k, value, act)
}

case string:
act, ok := actualValue.(string)
if !ok {
return fmt.Errorf("Expected value %v for %s was actually %v of type %T, not string",
v, k, actualValue, actualValue)
}

if act != value {
return fmt.Errorf("Expected %s value %v but got %v", k, value, act)
}

default:
return fmt.Errorf("Test case does not consider expected value %v for type %T", k, v)
}
}

if len(expected) != len(actual) {
return fmt.Errorf("expected %d fields, got %d", len(expected), len(actual))
}

return nil
}

func TestLongCLMNames(t *testing.T) {
for i, testData := range []struct {
loc CodeLocation
expected map[string]interface{}
}{
//0
{CodeLocation{42, "main.aFunction", "/usr/local/foo.go"},
map[string]interface{}{
AttributeCodeLineno: 42,
AttributeCodeFunction: "aFunction",
AttributeCodeNamespace: "main",
AttributeCodeFilepath: "/usr/local/foo.go",
}},
//1
{CodeLocation{42, "main.aFunction", "/usr/local/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo/foo.go"},
map[string]interface{}{
AttributeCodeLineno: 42,
AttributeCodeFunction: "aFunction",
AttributeCodeNamespace: "main",
}},
//2
{CodeLocation{42, "main.aFunctionLoremipsumdolorsitamet.consecteturadipiscingelit.seddoeiusmodtemporincididuntutlaboreetdoloremagnaaliqua.Utenimadminimveniamquisnostrudexercitationullamcolaborisnisiutaliquipexeacommodoconsequat.Duisauteiruredolorinreprehenderitinvoluptatevelitessecillumdoloreeufugiatnullapariatur.Excepteursintoccaecatcupidatatnonproidentsuntinculpaquiofficiadeseruntmollitanimidestlaborum", "/usr/local/foo.go"},
map[string]interface{}{
AttributeCodeLineno: 42,
AttributeCodeFunction: "Excepteursintoccaecatcupidatatnonproidentsuntinculpaquiofficiadeseruntmollitanimidestlaborum",
AttributeCodeFilepath: "/usr/local/foo.go",
}},
//3
{CodeLocation{42, "mainaFunctionLoremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididuntutlaboreetdoloremagnaaliquaUtenimadminimveniamquisnostrudexercitationullamcolaborisnisiutaliquipexeacommodoconsequatDuisauteiruredolorinreprehenderitinvoluptatevelitessecillumdoloreeufugiatnullapariaturExcepteursintoccaecatcupidatatnonproidentsuntinculpaquiofficiadeseruntmollitanimidestlaborum", "/usr/local/foo.go"},
map[string]interface{}{}},
//4
{CodeLocation{42, "", "/usr/local/foo.go"},
map[string]interface{}{}},
//5
{CodeLocation{42, "mainmainaFunctionLoremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididuntutlaboreetdoloremagnaaliquaUtenimadminimveniamquisnostrudexercitationullamcolaborisnisiutaliquipexeacommodoconsequatDuisauteiruredolorinreprehenderitinvoluptatevelitessecillumdoloreeufugiatnullapariaturExcepteursintoccaecatcupidatatnonproidentsuntinculpaquiofficiadeseruntmollitanimidestlaborum.aFunction", "/usr/local/foo.go"},
map[string]interface{}{
AttributeCodeLineno: 42,
AttributeCodeFunction: "aFunction",
AttributeCodeFilepath: "/usr/local/foo.go",
}},
//6
{CodeLocation{42, "mainmainaFunctionLoremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididuntutlaboreetdoloremagnaaliquaUtenimadminimveniamquisnostrudexercitationullamcolaborisnisiutaliquipexeacommodoconsequatDuisauteiruredolorinreprehenderitinvoluptatevelitessecillumdoloreeufugiatnullapariaturExcepteursintoccaecatcupidatatnonproidentsuntinculpaquiofficiadeseruntmollitanimidestlaborum.aFunction", "/usr/local/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaafoo.go"},
map[string]interface{}{}},
} {
actual := make(map[string]interface{})
reportCodeLevelMetrics(traceOptSet{
LocationOverride: &testData.loc,
PathPrefixes: []string{"xyzzy"},
}, nil, func(k, s string, v interface{}) {
if v == nil {
actual[k] = s
} else {
actual[k] = v
}
})
if err := attributeMapMatchesCLM(testData.expected, actual); err != nil {
t.Errorf("testcase %d: %v", i, err)
}
}
}