Skip to content

Commit 35e1174

Browse files
authored
fix(profiler): don't double compress non-delta profiles (#3918)
Our non-delta heap, block, and mutex profiles are getting double-compressed. That is, they are gzip-compressed, and if you decompress them then you get another gzip-compressed blob, which is in turn a compressed pprof file. This is due to a bug introduced by #3529. In that PR we reworked our compression logic in order to support re-compression (into zstd in particular). We incorrectly decided whether a given profile type is a delta profile by just checking whether there are delta values for the profile type, without checking whether delta profiling is actually enabled. As a result, when delta profiling is disabled we use the delta profling enabled logic, where we assume the input data is uncomprossed and then gzip-compress it.
1 parent 77122a1 commit 35e1174

File tree

2 files changed

+42
-1
lines changed

2 files changed

+42
-1
lines changed

profiler/profiler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ func newProfiler(opts ...Option) (*profiler, error) {
259259
types = append(types, executionTrace)
260260
}
261261
for _, pt := range types {
262-
isDelta := len(profileTypes[pt].DeltaValues) > 0
262+
isDelta := p.cfg.deltaProfiles && len(profileTypes[pt].DeltaValues) > 0
263263
in, out := compressionStrategy(pt, isDelta, p.cfg.compressionConfig)
264264
compressor, err := newCompressionPipeline(in, out)
265265
if err != nil {

profiler/profiler_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package profiler
77

88
import (
99
"bytes"
10+
"compress/gzip"
1011
"context"
1112
"encoding/json"
1213
"fmt"
@@ -35,6 +36,7 @@ import (
3536
"github.com/DataDog/dd-trace-go/v2/internal/traceprof"
3637
"github.com/DataDog/dd-trace-go/v2/internal/version"
3738

39+
pprofile "github.com/google/pprof/profile"
3840
"github.com/stretchr/testify/assert"
3941
"github.com/stretchr/testify/require"
4042
)
@@ -900,3 +902,42 @@ func TestMetricsProfileStopEarlyNoLog(t *testing.T) {
900902
}
901903
}
902904
}
905+
906+
func gzipDecompress(data []byte) ([]byte, error) {
907+
r, err := gzip.NewReader(bytes.NewReader(data))
908+
if err != nil {
909+
return nil, err
910+
}
911+
return io.ReadAll(r)
912+
}
913+
914+
func TestHeapProfileCompression(t *testing.T) {
915+
t.Run("delta", func(t *testing.T) { testHeapProfileCompression(t, true) })
916+
t.Run("non-delta", func(t *testing.T) { testHeapProfileCompression(t, false) })
917+
}
918+
919+
func testHeapProfileCompression(t *testing.T, delta bool) {
920+
profiles := startTestProfiler(t, 1,
921+
WithPeriod(10*time.Millisecond), WithProfileTypes(HeapProfile), WithDeltaProfiles(delta),
922+
)
923+
p := <-profiles
924+
attachment := "heap.pprof"
925+
if delta {
926+
attachment = "delta-heap.pprof"
927+
}
928+
data, ok := p.attachments[attachment]
929+
if !ok {
930+
t.Fatalf("no heap profile, got %s", p.event.Attachments)
931+
}
932+
decompressed, err := gzipDecompress(data)
933+
if err != nil {
934+
t.Fatalf("decompressing the heap profile failed: %s", err)
935+
}
936+
t.Logf("%x", decompressed[:16])
937+
// We assume the profile is gzip compressed. The pprof pacakge
938+
// can parse gzip-compressed profiles (it checks for the magic number).
939+
// So we should be able to parse the original data
940+
if _, err := pprofile.ParseData(data); err != nil {
941+
t.Fatalf("parsing profile data failed: %s", err)
942+
}
943+
}

0 commit comments

Comments
 (0)