Skip to content

Commit

Permalink
*: Fix address normalization (#806)
Browse files Browse the repository at this point in the history
* Add `start` to mapping key

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>

* Consider pre-normalized addresses during ingestion

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>

* Update comment according to suggestions

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
  • Loading branch information
kakkoyun authored Mar 24, 2022
1 parent f9a142b commit 182cd09
Show file tree
Hide file tree
Showing 27 changed files with 228 additions and 140 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ proto/generate:

.PHONY: proto/vendor
proto/vendor:
buf mod update
cd proto && buf mod update
mkdir -p proto/google/pprof
curl https://raw.githubusercontent.com/google/pprof/master/proto/profile.proto > proto/google/pprof/profile.proto

Expand Down
2 changes: 2 additions & 0 deletions gen/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
proto/go/google
proto/swagger/google
125 changes: 68 additions & 57 deletions gen/proto/go/parca/profilestore/v1alpha1/profilestore.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@
"$ref": "#/definitions/v1alpha1RawProfileSeries"
},
"title": "series is a set raw pprof profiles and accompanying labels"
},
"normalized": {
"type": "boolean",
"title": "normalized is a flag indicating if the addresses in the profile is normalized for position independent code"
}
},
"title": "WriteRawRequest writes a pprof profile for a given tenant"
Expand Down
18 changes: 6 additions & 12 deletions pkg/metastore/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,8 @@ const (
)

func MakeLocationKey(l *Location) []byte {
normalizedAddress := l.Address
if l.Mapping != nil {
// Normalizes address to handle address space randomization.
normalizedAddress -= l.Mapping.Start
}

linesLength := 0
if normalizedAddress == 0 {
if l.Address == 0 {
// Each line is a 16 byte Function ID + 8 byte line number
linesLength = len(l.Lines) * (16 + 8)
}
Expand All @@ -44,7 +38,7 @@ func MakeLocationKey(l *Location) []byte {
len(locationsKeyPrefix)+
// MappingID is 16 bytes
16+
// NormalizedAddress is 8 bytes
// Address is 8 bytes
8+
// IsFolded is encoded as 8 bytes
8+
Expand All @@ -54,20 +48,20 @@ func MakeLocationKey(l *Location) []byte {
if l.Mapping != nil {
copy(buf[len(locationsKeyPrefix):], l.Mapping.Id)
}
binary.BigEndian.PutUint64(buf[len(locationsKeyPrefix)+16:], normalizedAddress)
binary.BigEndian.PutUint64(buf[len(locationsKeyPrefix)+16:], l.Address)
if l.IsFolded {
// If IsFolded is false this means automatically that these 8 bytes are
// 0. This works out well as the key is byte aligned to the nearest 8
// bytes that way.
binary.BigEndian.PutUint64(buf[len(locationsKeyPrefix)+8+16:], 1)
}

// If the normalized address is 0, then the functions attached to the
// If the address is 0, then the functions attached to the
// location are not from a native binary, but instead from a dynamic
// runtime/language eg. ruby or python. In those cases we have no better
// uniqueness factor than the actual functions, and since there is no
// address there is no potential for asynchronously symbolizing.
if normalizedAddress == 0 {
if l.Address == 0 {
for i, line := range l.Lines {
copy(buf[len(locationsKeyPrefix)+16+8+8+24*i:], line.Function.Id)
binary.BigEndian.PutUint64(buf[len(locationsKeyPrefix)+16+8+8+24*i+8:], uint64(line.Line))
Expand Down Expand Up @@ -110,7 +104,7 @@ func MakeMappingKey(m *pb.Mapping) []byte {
// treated as the same mapping during merging.
}

buf := make([]byte, len(mappingKeyPrefix)+len(buildIDOrFile)+16)
buf := make([]byte, len(mappingKeyPrefix)+16+len(buildIDOrFile))
copy(buf, mappingKeyPrefix)
binary.BigEndian.PutUint64(buf[len(mappingKeyPrefix):], size)
binary.BigEndian.PutUint64(buf[len(mappingKeyPrefix)+8:], m.Offset)
Expand Down
8 changes: 4 additions & 4 deletions pkg/metastore/metastore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestMappingKeyBytes(t *testing.T) {
Offset: 2,
}

require.Equal(t, MakeMappingKey(m), []byte{
require.Equal(t, []byte{
0x76,
0x31,
0x2f,
Expand Down Expand Up @@ -67,7 +67,7 @@ func TestMappingKeyBytes(t *testing.T) {
0x0,
0x0,
0x2,
})
}, MakeMappingKey(m))

m = &pb.Mapping{
Start: 0,
Expand All @@ -76,7 +76,7 @@ func TestMappingKeyBytes(t *testing.T) {
File: "a",
}

require.Equal(t, MakeMappingKey(m), []byte{
require.Equal(t, []byte{
0x76,
0x31,
0x2f,
Expand Down Expand Up @@ -113,7 +113,7 @@ func TestMappingKeyBytes(t *testing.T) {
0x0,
0x2,
0x61,
})
}, MakeMappingKey(m))
}

func TestFunctionKeyBytes(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions pkg/metastore/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (s *sqlMetaStore) GetLocationByKey(ctx context.Context, lkey *Location) (*p
AND is_folded=?
AND lines=?
AND mapping_id=? `,
int64(k.NormalizedAddress), k.IsFolded, k.Lines, k.MappingID,
int64(k.Address), k.IsFolded, k.Lines, k.MappingID,
).Scan(&id, &address)
} else {
err = s.db.QueryRowContext(ctx,
Expand All @@ -136,7 +136,7 @@ func (s *sqlMetaStore) GetLocationByKey(ctx context.Context, lkey *Location) (*p
AND mapping_id IS NULL
AND is_folded=?
AND lines=?`,
int64(k.NormalizedAddress), k.IsFolded, k.Lines,
int64(k.Address), k.IsFolded, k.Lines,
).Scan(&id, &address)
}
if err != nil {
Expand Down Expand Up @@ -643,7 +643,7 @@ func (s *sqlMetaStore) CreateLocation(ctx context.Context, l *Location) ([]byte,
defer stmt.Close()

f = func() error {
_, err = stmt.ExecContext(ctx, id.String(), int64(l.Address), l.IsFolded, mID.String(), int64(k.NormalizedAddress), k.Lines)
_, err = stmt.ExecContext(ctx, id.String(), int64(l.Address), l.IsFolded, mID.String(), int64(k.Address), k.Lines)
return err
}
} else {
Expand All @@ -656,7 +656,7 @@ func (s *sqlMetaStore) CreateLocation(ctx context.Context, l *Location) ([]byte,
defer stmt.Close()

f = func() error {
_, err = stmt.ExecContext(ctx, id.String(), int64(l.Address), l.IsFolded, int64(k.NormalizedAddress), k.Lines)
_, err = stmt.ExecContext(ctx, id.String(), int64(l.Address), l.IsFolded, int64(k.Address), k.Lines)
return err
}
}
Expand Down
Loading

0 comments on commit 182cd09

Please sign in to comment.