diff --git a/profcheck/check.go b/profcheck/check.go index 31c77c6..804204b 100644 --- a/profcheck/check.go +++ b/profcheck/check.go @@ -19,6 +19,7 @@ package profcheck import ( "errors" "fmt" + "slices" profiles "go.opentelemetry.io/proto/otlp/profiles/v1development" "google.golang.org/protobuf/proto" @@ -212,7 +213,17 @@ func (c ConformanceChecker) checkMappingTable(mappingTable []*profiles.Mapping, if err := checkZeroVal(mappingTable); err != nil { errs = errors.Join(errs, err) } - for idx, m := range mappingTable { + + type uniqMapping struct { + filenameStrIdx int32 + attrIdxs []int32 + memStart uint64 + memLimit uint64 + fileOffset uint64 + } + var uniqMappings []uniqMapping + + for idx, m := range mappingTable[1:] { if err := c.checkIndex(len(dict.StringTable), m.FilenameStrindex); err != nil { errs = errors.Join(errs, prefixErrorf(err, "[%d].filename_strindex", idx)) } @@ -222,8 +233,30 @@ func (c ConformanceChecker) checkMappingTable(mappingTable []*profiles.Mapping, if !(m.MemoryStart == 0 && m.MemoryLimit == 0) && !(m.MemoryStart < m.MemoryLimit) { errs = errors.Join(errs, fmt.Errorf("[%d]: memory_start=%016x, memory_limit=%016x: must be both zero or start < limit", idx, m.MemoryStart, m.MemoryLimit)) } + if c.CheckDictionaryDuplicates { + newMapping := uniqMapping{ + filenameStrIdx: m.FilenameStrindex, + attrIdxs: m.AttributeIndices, + memStart: m.MemoryStart, + memLimit: m.MemoryLimit, + fileOffset: m.FileOffset, + } + // Sort the slice of IDs before comparing it. So we also make sure + // only sorted IDs are added to uniqMappings. + slices.Sort(newMapping.attrIdxs) + if slices.ContainsFunc(uniqMappings, func(e uniqMapping) bool { + return newMapping.filenameStrIdx == e.filenameStrIdx && + slices.Equal(newMapping.attrIdxs, e.attrIdxs) && + newMapping.memStart == e.memStart && + newMapping.memLimit == e.memLimit && + newMapping.fileOffset == e.fileOffset + }) { + errs = errors.Join(errs, fmt.Errorf("duplicate mapping at index %d: %v", idx, m)) + continue + } + uniqMappings = append(uniqMappings, newMapping) + } } - // TODO: Add optional uniqueness check. // TODO: Add optional unreferenced entries check. return errs } diff --git a/profcheck/check_test.go b/profcheck/check_test.go index 89387d8..33a601a 100644 --- a/profcheck/check_test.go +++ b/profcheck/check_test.go @@ -33,287 +33,349 @@ func TestCheckConformance(t *testing.T) { disableDupesCheck bool checkSampleShapes bool wantErr string - }{{ - desc: "no profiles", - data: &profiles.ProfilesData{}, - wantErr: "resource profiles are empty", - }, { - desc: "minimal valid profile", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{}}, - }}, - }}, + }{ + { + desc: "no profiles", + data: &profiles.ProfilesData{}, + wantErr: "resource profiles are empty", }, - wantErr: "", - }, { - desc: "no scope profiles", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{}}, + { + desc: "minimal valid profile", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{}}, + }}, + }}, + }, + wantErr: "", }, - wantErr: "resource profiles has no scope profiles", - }, { - desc: "no profiles in scope", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{}}, - }}, + { + desc: "no scope profiles", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{}}, + }, + wantErr: "resource profiles has no scope profiles", }, - wantErr: "scope profiles has no profiles", - }, { - desc: "no empty string at pos 0", - data: &profiles.ProfilesData{ - Dictionary: zeroDictWithStringTable([]string{"a"}), - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{}}, + { + desc: "no profiles in scope", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{}}, }}, - }}, + }, + wantErr: "scope profiles has no profiles", }, - wantErr: "must have empty string at index 0", - }, { - desc: "duplicate string", - data: &profiles.ProfilesData{ - Dictionary: zeroDictWithStringTable([]string{"", "a", "b", "a"}), - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{}}, + { + desc: "no empty string at pos 0", + data: &profiles.ProfilesData{ + Dictionary: zeroDictWithStringTable([]string{"a"}), + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{}}, + }}, }}, - }}, + }, + wantErr: "must have empty string at index 0", }, - wantErr: "duplicate string", - }, { - desc: "duplicate string (disabled check)", - data: &profiles.ProfilesData{ - Dictionary: zeroDictWithStringTable([]string{"", "a", "b", "a"}), - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{}}, + { + desc: "duplicate string", + data: &profiles.ProfilesData{ + Dictionary: zeroDictWithStringTable([]string{"", "a", "b", "a"}), + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{}}, + }}, }}, - }}, + }, + wantErr: "duplicate string", }, - disableDupesCheck: true, - wantErr: "", - }, { - desc: "duplicate attribute key in location", - data: &profiles.ProfilesData{ - Dictionary: &profiles.ProfilesDictionary{ - MappingTable: []*profiles.Mapping{{}}, - LocationTable: []*profiles.Location{ - {}, - {AttributeIndices: []int32{1, 2}}, + { + desc: "duplicate mapping", + data: &profiles.ProfilesData{ + Dictionary: &profiles.ProfilesDictionary{ + MappingTable: []*profiles.Mapping{ + {}, + { + FilenameStrindex: 1, + AttributeIndices: []int32{1, 2}, + MemoryStart: 0x4000, + MemoryLimit: 0x4200, + }, + { + FilenameStrindex: 1, + AttributeIndices: []int32{2, 1}, + MemoryStart: 0x4000, + MemoryLimit: 0x4200, + }, + }, + LocationTable: []*profiles.Location{{}}, + FunctionTable: []*profiles.Function{{}}, + LinkTable: []*profiles.Link{{}}, + StringTable: []string{"", "filename", "attr1", "attr2"}, + AttributeTable: []*profiles.KeyValueAndUnit{ + {}, + { + KeyStrindex: 1, Value: makeAnyValue("v1"), + }, + { + KeyStrindex: 2, Value: makeAnyValue("v2"), + }, + }, + StackTable: []*profiles.Stack{{}}, }, - FunctionTable: []*profiles.Function{{}}, - LinkTable: []*profiles.Link{{}}, - StringTable: []string{"", "k1"}, - AttributeTable: []*profiles.KeyValueAndUnit{ - {}, - {KeyStrindex: 1, Value: makeAnyValue("v1")}, - {KeyStrindex: 1, Value: makeAnyValue("v2")}, - }, - StackTable: []*profiles.Stack{{}}, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{}}, + }}, + }}, }, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{}}, + wantErr: "duplicate mapping", + }, + { + desc: "duplicate string (disabled check)", + data: &profiles.ProfilesData{ + Dictionary: zeroDictWithStringTable([]string{"", "a", "b", "a"}), + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{}}, + }}, }}, - }}, + }, + disableDupesCheck: true, + wantErr: "", }, - wantErr: `duplicate key "k1"`, - }, { - desc: "timestamp before start", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - TimeUnixNano: 100, - DurationNano: 10, - Samples: []*profiles.Sample{{ - TimestampsUnixNano: []uint64{99}, + { + desc: "duplicate attribute key in location", + data: &profiles.ProfilesData{ + Dictionary: &profiles.ProfilesDictionary{ + MappingTable: []*profiles.Mapping{{}}, + LocationTable: []*profiles.Location{ + {}, + {AttributeIndices: []int32{1, 2}}, + }, + FunctionTable: []*profiles.Function{{}}, + LinkTable: []*profiles.Link{{}}, + StringTable: []string{"", "k1"}, + AttributeTable: []*profiles.KeyValueAndUnit{ + {}, + {KeyStrindex: 1, Value: makeAnyValue("v1")}, + {KeyStrindex: 1, Value: makeAnyValue("v2")}, + }, + StackTable: []*profiles.Stack{{}}, + }, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{}}, + }}, + }}, + }, + wantErr: `duplicate key "k1"`, + }, + { + desc: "timestamp before start", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + TimeUnixNano: 100, + DurationNano: 10, + Samples: []*profiles.Sample{{ + TimestampsUnixNano: []uint64{99}, + }}, }}, }}, }}, - }}, + }, + wantErr: "timestamps_unix_nano[0]=99 is outside profile time range [100, 110)", }, - wantErr: "timestamps_unix_nano[0]=99 is outside profile time range [100, 110)", - }, { - desc: "timestamp at start", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - TimeUnixNano: 100, - DurationNano: 10, - Samples: []*profiles.Sample{{ - TimestampsUnixNano: []uint64{100}, + { + desc: "timestamp at start", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + TimeUnixNano: 100, + DurationNano: 10, + Samples: []*profiles.Sample{{ + TimestampsUnixNano: []uint64{100}, + }}, }}, }}, }}, - }}, + }, + wantErr: "", }, - wantErr: "", - }, { - desc: "timestamp at end (exclusive)", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - TimeUnixNano: 100, - DurationNano: 10, - Samples: []*profiles.Sample{{ - TimestampsUnixNano: []uint64{110}, + { + desc: "timestamp at end (exclusive)", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + TimeUnixNano: 100, + DurationNano: 10, + Samples: []*profiles.Sample{{ + TimestampsUnixNano: []uint64{110}, + }}, }}, }}, }}, - }}, + }, + wantErr: "timestamps_unix_nano[0]=110 is outside profile time range [100, 110)", }, - wantErr: "timestamps_unix_nano[0]=110 is outside profile time range [100, 110)", - }, { - desc: "timestamp after end", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - TimeUnixNano: 100, - DurationNano: 10, - Samples: []*profiles.Sample{{ - TimestampsUnixNano: []uint64{111}, + { + desc: "timestamp after end", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + TimeUnixNano: 100, + DurationNano: 10, + Samples: []*profiles.Sample{{ + TimestampsUnixNano: []uint64{111}, + }}, }}, }}, }}, - }}, + }, + wantErr: "timestamps_unix_nano[0]=111 is outside profile time range [100, 110)", }, - wantErr: "timestamps_unix_nano[0]=111 is outside profile time range [100, 110)", - }, { - desc: "sample with no values and no timestamps", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - Samples: []*profiles.Sample{{}}, + { + desc: "sample with no values and no timestamps", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + Samples: []*profiles.Sample{{}}, + }}, }}, }}, - }}, + }, + wantErr: "sample must have at least one values or timestamps_unix_nano entry", }, - wantErr: "sample must have at least one values or timestamps_unix_nano entry", - }, { - desc: "sample with values and timestamps length mismatch", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - TimeUnixNano: 100, - DurationNano: 10, - Samples: []*profiles.Sample{{ - Values: []int64{1}, - TimestampsUnixNano: []uint64{100, 101}, + { + desc: "sample with values and timestamps length mismatch", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + TimeUnixNano: 100, + DurationNano: 10, + Samples: []*profiles.Sample{{ + Values: []int64{1}, + TimestampsUnixNano: []uint64{100, 101}, + }}, }}, }}, }}, - }}, + }, + wantErr: "values (len=1) and timestamps_unix_nano (len=2) must contain the same number of elements", }, - wantErr: "values (len=1) and timestamps_unix_nano (len=2) must contain the same number of elements", - }, { - desc: "sample with values only", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - Samples: []*profiles.Sample{{ - Values: []int64{1}, + { + desc: "sample with values only", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + Samples: []*profiles.Sample{{ + Values: []int64{1}, + }}, }}, }}, }}, - }}, + }, + wantErr: "", }, - wantErr: "", - }, { - desc: "sample with timestamps only", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - TimeUnixNano: 100, - DurationNano: 10, - Samples: []*profiles.Sample{{ - TimestampsUnixNano: []uint64{100}, + { + desc: "sample with timestamps only", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + TimeUnixNano: 100, + DurationNano: 10, + Samples: []*profiles.Sample{{ + TimestampsUnixNano: []uint64{100}, + }}, }}, }}, }}, - }}, + }, + wantErr: "", }, - wantErr: "", - }, { - desc: "sample with values and timestamps matching", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - TimeUnixNano: 100, - DurationNano: 10, - Samples: []*profiles.Sample{{ - Values: []int64{1}, - TimestampsUnixNano: []uint64{100}, + { + desc: "sample with values and timestamps matching", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + TimeUnixNano: 100, + DurationNano: 10, + Samples: []*profiles.Sample{{ + Values: []int64{1}, + TimestampsUnixNano: []uint64{100}, + }}, }}, }}, }}, - }}, + }, + wantErr: "", }, - wantErr: "", - }, { - desc: "mixed sample types", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - TimeUnixNano: 100, - DurationNano: 10, - Samples: []*profiles.Sample{{ - Values: []int64{1}, - }, { - TimestampsUnixNano: []uint64{100}, + { + desc: "mixed sample types", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + TimeUnixNano: 100, + DurationNano: 10, + Samples: []*profiles.Sample{{ + Values: []int64{1}, + }, { + TimestampsUnixNano: []uint64{100}, + }}, }}, }}, }}, - }}, + }, + checkSampleShapes: true, + wantErr: "does not match expected sample shape", }, - checkSampleShapes: true, - wantErr: "does not match expected sample shape", - }, { - desc: "mixed sample types (disabled check)", - data: &profiles.ProfilesData{ - Dictionary: zeroDictionary, - ResourceProfiles: []*profiles.ResourceProfiles{{ - ScopeProfiles: []*profiles.ScopeProfiles{{ - Profiles: []*profiles.Profile{{ - TimeUnixNano: 100, - DurationNano: 10, - Samples: []*profiles.Sample{{ - Values: []int64{1}, - }, { - TimestampsUnixNano: []uint64{100}, + { + desc: "mixed sample types (disabled check)", + data: &profiles.ProfilesData{ + Dictionary: zeroDictionary, + ResourceProfiles: []*profiles.ResourceProfiles{{ + ScopeProfiles: []*profiles.ScopeProfiles{{ + Profiles: []*profiles.Profile{{ + TimeUnixNano: 100, + DurationNano: 10, + Samples: []*profiles.Sample{{ + Values: []int64{1}, + }, { + TimestampsUnixNano: []uint64{100}, + }}, }}, }}, }}, - }}, + }, + checkSampleShapes: false, + wantErr: "", }, - checkSampleShapes: false, - wantErr: "", - }} { + } { t.Run(tc.desc, func(t *testing.T) { c := ConformanceChecker{CheckDictionaryDuplicates: !tc.disableDupesCheck, CheckSampleTimestampShape: tc.checkSampleShapes} err := c.Check(tc.data)