diff --git a/Dockerfiles/agent/entrypoint.d/agent b/Dockerfiles/agent/entrypoint.d/agent index 7112494fa8ffa..ca7fc08582253 100755 --- a/Dockerfiles/agent/entrypoint.d/agent +++ b/Dockerfiles/agent/entrypoint.d/agent @@ -2,5 +2,5 @@ set -euo pipefail "Running entrypoint.d/agent" -#exec /root/go/bin/dlv --listen=:2345 --headless=true --log=true --log-output=debugger,debuglineerr,gdbwire,lldbout,rpc --accept-multiclient --api-version=2 exec /opt/datadog-agent/bin/agent/agent -- run -/opt/datadog-agent/bin/agent run +exec /root/go/bin/dlv --listen=:2345 --headless=true --log=true --log-output=debugger,debuglineerr,gdbwire,lldbout,rpc --accept-multiclient --api-version=2 exec /opt/datadog-agent/bin/agent/agent -- run +#/opt/datadog-agent/bin/agent run diff --git a/cmd/agent/subcommands/run/command.go b/cmd/agent/subcommands/run/command.go index d811a3aae5fc8..7ee2ae74cceec 100644 --- a/cmd/agent/subcommands/run/command.go +++ b/cmd/agent/subcommands/run/command.go @@ -329,7 +329,6 @@ func getSharedFxOption() fx.Option { params.Options.EnabledFeatures = pkgforwarder.SetFeature(params.Options.EnabledFeatures, pkgforwarder.CoreFeatures) return params }), - fx.Provide(cache.NewKeyedStringInterner), dogstatsd.Bundle, otelcol.Bundle, rcclient.Module, diff --git a/cmd/cluster-agent-cloudfoundry/subcommands/run/command.go b/cmd/cluster-agent-cloudfoundry/subcommands/run/command.go index 6eaa80cc8620e..8ca2a376089da 100644 --- a/cmd/cluster-agent-cloudfoundry/subcommands/run/command.go +++ b/cmd/cluster-agent-cloudfoundry/subcommands/run/command.go @@ -97,7 +97,7 @@ func run(log log.Component, config config.Component, forwarder defaultforwarder. opts := aggregator.DefaultAgentDemultiplexerOptions() opts.UseEventPlatformForwarder = false opts.UseOrchestratorForwarder = false - demux := aggregator.InitAndStartAgentDemultiplexer(log, forwarder, opts, hname) + demux := aggregator.InitAndStartAgentDemultiplexer(log, forwarder, opts, hname, nil) demux.AddAgentStartupTelemetry(fmt.Sprintf("%s - Datadog Cluster Agent", version.AgentVersion)) pkglog.Infof("Datadog Cluster Agent is now running.") diff --git a/cmd/cluster-agent/subcommands/start/command.go b/cmd/cluster-agent/subcommands/start/command.go index a18c6a2665127..e432f2608433d 100644 --- a/cmd/cluster-agent/subcommands/start/command.go +++ b/cmd/cluster-agent/subcommands/start/command.go @@ -189,7 +189,7 @@ func start(log log.Component, config config.Component, telemetry telemetry.Compo // Serving stale data is better than serving no data at all. opts := aggregator.DefaultAgentDemultiplexerOptions() opts.UseEventPlatformForwarder = false - demux := aggregator.InitAndStartAgentDemultiplexer(log, forwarder, opts, hname) + demux := aggregator.InitAndStartAgentDemultiplexer(log, forwarder, opts, hname, nil) demux.AddAgentStartupTelemetry(fmt.Sprintf("%s - Datadog Cluster Agent", version.AgentVersion)) le, err := leaderelection.GetLeaderEngine() diff --git a/cmd/dogstatsd/subcommands/start/command.go b/cmd/dogstatsd/subcommands/start/command.go index 8c80edc6d9045..f9b41861a822a 100644 --- a/cmd/dogstatsd/subcommands/start/command.go +++ b/cmd/dogstatsd/subcommands/start/command.go @@ -201,7 +201,7 @@ func RunAgent(ctx context.Context, cliParams *CLIParams, config config.Component hname = "" } log.Debugf("Using hostname: %s", hname) - demux := aggregator.InitAndStartAgentDemultiplexer(log, forwarder, opts, hname) + demux := aggregator.InitAndStartAgentDemultiplexer(log, forwarder, opts, hname, nil) demux.AddAgentStartupTelemetry(version.AgentVersion) // setup the metadata collector diff --git a/cmd/otel-agent/main.go b/cmd/otel-agent/main.go index d600878c58e00..26da255873c32 100644 --- a/cmd/otel-agent/main.go +++ b/cmd/otel-agent/main.go @@ -108,5 +108,5 @@ func newDemultiplexer(logcomp corelog.Component, cfg config.Component, fwd defau opts.EnableNoAggregationPipeline = cfg.GetBool("dogstatsd_no_aggregation_pipeline") opts.UseDogstatsdContextLimiter = true opts.DogstatsdMaxMetricsTags = cfg.GetInt("dogstatsd_max_metrics_tags") - return aggregator.InitAndStartAgentDemultiplexer(logcomp, fwd, opts, host) + return aggregator.InitAndStartAgentDemultiplexer(logcomp, fwd, opts, host, nil) } diff --git a/cmd/security-agent/subcommands/start/command.go b/cmd/security-agent/subcommands/start/command.go index 1b33271697917..51087734a99b2 100644 --- a/cmd/security-agent/subcommands/start/command.go +++ b/cmd/security-agent/subcommands/start/command.go @@ -217,7 +217,7 @@ func RunAgent(ctx context.Context, log log.Component, config config.Component, s opts := aggregator.DefaultAgentDemultiplexerOptions() opts.UseEventPlatformForwarder = false opts.UseOrchestratorForwarder = false - demux := aggregator.InitAndStartAgentDemultiplexer(log, forwarder, opts, hostnameDetected) + demux := aggregator.InitAndStartAgentDemultiplexer(log, forwarder, opts, hostnameDetected, nil) demux.AddAgentStartupTelemetry(fmt.Sprintf("%s - Datadog Security Agent", version.AgentVersion)) stopper = startstop.NewSerialStopper() diff --git a/comp/aggregator/demultiplexer/demultiplexer.go b/comp/aggregator/demultiplexer/demultiplexer.go index 97e5bb9fc1355..8045d08fa5ca7 100644 --- a/comp/aggregator/demultiplexer/demultiplexer.go +++ b/comp/aggregator/demultiplexer/demultiplexer.go @@ -33,7 +33,8 @@ func newDemultiplexer(deps dependencies) (Component, error) { deps.Log, deps.SharedForwarder, deps.Params.Options, - hostnameDetected) + hostnameDetected, + nil) return demux, nil } diff --git a/comp/dogstatsd/bundle.go b/comp/dogstatsd/bundle.go index 0c42fbf96c82d..e15e4ba196fc4 100644 --- a/comp/dogstatsd/bundle.go +++ b/comp/dogstatsd/bundle.go @@ -9,6 +9,7 @@ import ( "github.com/DataDog/datadog-agent/comp/dogstatsd/replay" "github.com/DataDog/datadog-agent/comp/dogstatsd/server" "github.com/DataDog/datadog-agent/comp/dogstatsd/serverDebug" + "github.com/DataDog/datadog-agent/pkg/util/cache" "github.com/DataDog/datadog-agent/pkg/util/fxutil" ) @@ -19,6 +20,7 @@ var Bundle = fxutil.Bundle( serverDebug.Module, replay.Module, server.Module, + cache.Module, ) // MockBundle defines the mock fx options for this bundle. @@ -26,4 +28,5 @@ var MockBundle = fxutil.Bundle( serverDebug.MockModule, server.MockModule, replay.Module, + cache.Module, ) diff --git a/comp/dogstatsd/server/batch.go b/comp/dogstatsd/server/batch.go index ba4c73c972d7c..bfbd91f9529f1 100644 --- a/comp/dogstatsd/server/batch.go +++ b/comp/dogstatsd/server/batch.go @@ -58,8 +58,7 @@ type batcher struct { noAggPipelineEnabled bool // Retain intern'd objects long enough to serialize them back out. - retentions map[cache.Refcounted]int32 - cache.InternRetainer + retentions *cache.RetainerBlock } // Use fastrange instead of a modulo for better performance. @@ -113,7 +112,7 @@ func newBatcher(demux aggregator.DemultiplexerWithAggregator) *batcher { keyGenerator: ckey.NewKeyGenerator(), noAggPipelineEnabled: demux.Options().EnableNoAggregationPipeline, - retentions: make(map[cache.Refcounted]int32), + retentions: cache.NewRetainerBlock(), } } @@ -183,6 +182,7 @@ func (b *batcher) appendLateSample(sample metrics.MetricSample) { return } + // TODO: Fix reference work here. if b.samplesWithTsCount == len(b.samplesWithTs) { b.flushSamplesWithTs() } @@ -222,7 +222,7 @@ func (b *batcher) flushSamplesWithTs() { // the demux for processing. Do this last so our worst mistake is to // retain things too long instead of long enough. func (b *batcher) forwardRetentions() { - b.demux.TakeRetentions(b, "batcher") + b.demux.TakeRetentions(b.Retainer(), "batcher") } // flush pushes all batched metrics to the aggregator. @@ -258,21 +258,6 @@ func (b *batcher) flush() { b.forwardRetentions() } -// Interner Retention -// -------- --------- - -func (b *batcher) Reference(obj cache.Refcounted) { - b.retentions[obj] += 1 -} -func (b *batcher) ReleaseAllWith(callback func(obj cache.Refcounted, count int32)) { - for k, v := range b.retentions { - callback(k, v) - delete(b.retentions, k) - } -} - -func (b *batcher) ReleaseAll() { - b.ReleaseAllWith(func(obj cache.Refcounted, count int32) { - obj.Release(count) - }) +func (b *batcher) Retainer() cache.InternRetainer { + return b.retentions } diff --git a/comp/dogstatsd/server/convert_bench_test.go b/comp/dogstatsd/server/convert_bench_test.go index 6bf7c4acf7a35..75865f2a21110 100644 --- a/comp/dogstatsd/server/convert_bench_test.go +++ b/comp/dogstatsd/server/convert_bench_test.go @@ -7,6 +7,7 @@ package server import ( "fmt" + "github.com/DataDog/datadog-agent/pkg/util/cache" "testing" "github.com/DataDog/datadog-agent/comp/core/config" @@ -33,7 +34,9 @@ var ( func runParseMetricBenchmark(b *testing.B, multipleValues bool) { cfg := fxutil.Test[config.Component](b, config.MockModule) - parser := newParser(cfg, newFloat64ListPool()) + interner := cache.NewKeyedStringInternerMemOnly(16384) + parser := newParser(cfg, newFloat64ListPool(), interner) + retainerCtx := cache.NewInternerContext(interner, "", &cache.SmallRetainer{}) conf := enrichConfig{ defaultHostname: "default-hostname", @@ -48,7 +51,7 @@ func runParseMetricBenchmark(b *testing.B, multipleValues bool) { for n := 0; n < sb.N; n++ { - parsed, err := parser.parseMetricSample(rawSample) + parsed, err := parser.parseMetricSample(rawSample, retainerCtx) if err != nil { continue } diff --git a/comp/dogstatsd/server/enrich_test.go b/comp/dogstatsd/server/enrich_test.go index 1c571897c551a..daa58c9f9694f 100644 --- a/comp/dogstatsd/server/enrich_test.go +++ b/comp/dogstatsd/server/enrich_test.go @@ -7,6 +7,7 @@ package server import ( "fmt" + "github.com/DataDog/datadog-agent/pkg/util/cache" "testing" "github.com/DataDog/datadog-agent/comp/core/config" @@ -31,10 +32,16 @@ var ( } ) +func internerAndContext() (*cache.KeyedInterner, cache.InternerContext) { + interner := cache.NewKeyedStringInternerMemOnly(16384) + return interner, cache.NewInternerContext(interner, "", &cache.SmallRetainer{}) +} + func parseAndEnrichSingleMetricMessage(t *testing.T, message []byte, conf enrichConfig) (metrics.MetricSample, error) { cfg := fxutil.Test[config.Component](t, config.MockModule) - parser := newParser(cfg, newFloat64ListPool()) - parsed, err := parser.parseMetricSample(message) + in, ctx := internerAndContext() + parser := newParser(cfg, newFloat64ListPool(), in) + parsed, err := parser.parseMetricSample(message, ctx) if err != nil { return metrics.MetricSample{}, err } @@ -49,8 +56,9 @@ func parseAndEnrichSingleMetricMessage(t *testing.T, message []byte, conf enrich func parseAndEnrichMultipleMetricMessage(t *testing.T, message []byte, conf enrichConfig) ([]metrics.MetricSample, error) { cfg := fxutil.Test[config.Component](t, config.MockModule) - parser := newParser(cfg, newFloat64ListPool()) - parsed, err := parser.parseMetricSample(message) + in, ctx := internerAndContext() + parser := newParser(cfg, newFloat64ListPool(), in) + parsed, err := parser.parseMetricSample(message, ctx) if err != nil { return []metrics.MetricSample{}, err } @@ -61,8 +69,9 @@ func parseAndEnrichMultipleMetricMessage(t *testing.T, message []byte, conf enri func parseAndEnrichServiceCheckMessage(t *testing.T, message []byte, conf enrichConfig) (*servicecheck.ServiceCheck, error) { cfg := fxutil.Test[config.Component](t, config.MockModule) - parser := newParser(cfg, newFloat64ListPool()) - parsed, err := parser.parseServiceCheck(message) + in, ctx := internerAndContext() + parser := newParser(cfg, newFloat64ListPool(), in) + parsed, err := parser.parseServiceCheck(message, ctx) if err != nil { return nil, err } @@ -71,8 +80,9 @@ func parseAndEnrichServiceCheckMessage(t *testing.T, message []byte, conf enrich func parseAndEnrichEventMessage(t *testing.T, message []byte, conf enrichConfig) (*event.Event, error) { cfg := fxutil.Test[config.Component](t, config.MockModule) - parser := newParser(cfg, newFloat64ListPool()) - parsed, err := parser.parseEvent(message) + in, ctx := internerAndContext() + parser := newParser(cfg, newFloat64ListPool(), in) + parsed, err := parser.parseEvent(message, ctx) if err != nil { return nil, err } @@ -959,8 +969,9 @@ func TestMetricBlocklistShouldBlock(t *testing.T) { } cfg := fxutil.Test[config.Component](t, config.MockModule) - parser := newParser(cfg, newFloat64ListPool()) - parsed, err := parser.parseMetricSample(message) + in, ctx := internerAndContext() + parser := newParser(cfg, newFloat64ListPool(), in) + parsed, err := parser.parseMetricSample(message, ctx) assert.NoError(t, err) samples := []metrics.MetricSample{} samples = enrichMetricSample(samples, parsed, "", conf) @@ -976,8 +987,9 @@ func TestServerlessModeShouldSetEmptyHostname(t *testing.T) { message := []byte("custom.metric.a:21|ms") cfg := fxutil.Test[config.Component](t, config.MockModule) - parser := newParser(cfg, newFloat64ListPool()) - parsed, err := parser.parseMetricSample(message) + in, ctx := internerAndContext() + parser := newParser(cfg, newFloat64ListPool(), in) + parsed, err := parser.parseMetricSample(message, ctx) assert.NoError(t, err) samples := []metrics.MetricSample{} samples = enrichMetricSample(samples, parsed, "", conf) @@ -996,8 +1008,9 @@ func TestMetricBlocklistShouldNotBlock(t *testing.T) { defaultHostname: "default", } cfg := fxutil.Test[config.Component](t, config.MockModule) - parser := newParser(cfg, newFloat64ListPool()) - parsed, err := parser.parseMetricSample(message) + in, ctx := internerAndContext() + parser := newParser(cfg, newFloat64ListPool(), in) + parsed, err := parser.parseMetricSample(message, ctx) assert.NoError(t, err) samples := []metrics.MetricSample{} samples = enrichMetricSample(samples, parsed, "", conf) diff --git a/comp/dogstatsd/server/parse_events_test.go b/comp/dogstatsd/server/parse_events_test.go index 8687fdc9b9a17..1c43369a04512 100644 --- a/comp/dogstatsd/server/parse_events_test.go +++ b/comp/dogstatsd/server/parse_events_test.go @@ -6,6 +6,7 @@ package server import ( + "github.com/DataDog/datadog-agent/pkg/util/cache" "testing" "github.com/DataDog/datadog-agent/comp/core/config" @@ -16,8 +17,10 @@ import ( func parseEvent(t *testing.T, rawEvent []byte) (dogstatsdEvent, error) { cfg := fxutil.Test[config.Component](t, config.MockModule) - parser := newParser(cfg, newFloat64ListPool()) - return parser.parseEvent(rawEvent) + kint := cache.NewKeyedStringInternerForTest() + + parser := newParser(cfg, newFloat64ListPool(), kint) + return parser.parseEvent(rawEvent, cache.NewInternerContext(kint, "", &cache.SmallRetainer{})) } func TestEventMinimal(t *testing.T) { diff --git a/comp/dogstatsd/server/parse_metrics_test.go b/comp/dogstatsd/server/parse_metrics_test.go index 8d3c786bc2d77..a4de5982db4c2 100644 --- a/comp/dogstatsd/server/parse_metrics_test.go +++ b/comp/dogstatsd/server/parse_metrics_test.go @@ -6,6 +6,7 @@ package server import ( + "github.com/DataDog/datadog-agent/pkg/util/cache" "testing" "time" @@ -21,8 +22,10 @@ func parseMetricSample(t *testing.T, overrides map[string]any, rawSample []byte) config.MockModule, fx.Replace(config.MockParams{Overrides: overrides}), )) - parser := newParser(cfg, newFloat64ListPool()) - return parser.parseMetricSample(rawSample) + kint := cache.NewKeyedStringInternerForTest() + + parser := newParser(cfg, newFloat64ListPool(), kint) + return parser.parseMetricSample(rawSample, cache.NewInternerContext(kint, "", &cache.SmallRetainer{})) } const epsilon = 0.00001 diff --git a/comp/dogstatsd/server/parse_service_checks_test.go b/comp/dogstatsd/server/parse_service_checks_test.go index 4433e9076b5fe..587718cf1738d 100644 --- a/comp/dogstatsd/server/parse_service_checks_test.go +++ b/comp/dogstatsd/server/parse_service_checks_test.go @@ -6,6 +6,7 @@ package server import ( + "github.com/DataDog/datadog-agent/pkg/util/cache" "testing" "github.com/DataDog/datadog-agent/comp/core/config" @@ -16,8 +17,9 @@ import ( func parseServiceCheck(t *testing.T, rawServiceCheck []byte) (dogstatsdServiceCheck, error) { cfg := fxutil.Test[config.Component](t, config.MockModule) - parser := newParser(cfg, newFloat64ListPool()) - return parser.parseServiceCheck(rawServiceCheck) + kint := cache.NewKeyedStringInternerForTest() + parser := newParser(cfg, newFloat64ListPool(), kint) + return parser.parseServiceCheck(rawServiceCheck, cache.NewInternerContext(kint, "", &cache.SmallRetainer{})) } func TestServiceCheckMinimal(t *testing.T) { diff --git a/comp/dogstatsd/server/parse_test.go b/comp/dogstatsd/server/parse_test.go index 361f5df0e633a..a1adcd3eb24f5 100644 --- a/comp/dogstatsd/server/parse_test.go +++ b/comp/dogstatsd/server/parse_test.go @@ -6,6 +6,7 @@ package server import ( + "github.com/DataDog/datadog-agent/pkg/util/cache" "strconv" "testing" @@ -40,18 +41,20 @@ func TestIdentifyRandomString(t *testing.T) { func TestParseTags(t *testing.T) { cfg := fxutil.Test[config.Component](t, config.MockModule) - p := newParser(cfg, newFloat64ListPool()) + kint := cache.NewKeyedStringInternerForTest() + p := newParser(cfg, newFloat64ListPool(), kint) rawTags := []byte("tag:test,mytag,good:boy") - tags := p.parseTags(rawTags) + tags := p.parseTags(rawTags, cache.NewInternerContext(kint, "", &cache.SmallRetainer{})) expectedTags := []string{"tag:test", "mytag", "good:boy"} assert.ElementsMatch(t, expectedTags, tags) } func TestParseTagsEmpty(t *testing.T) { cfg := fxutil.Test[config.Component](t, config.MockModule) - p := newParser(cfg, newFloat64ListPool()) + kint := cache.NewKeyedStringInternerForTest() + p := newParser(cfg, newFloat64ListPool(), kint) rawTags := []byte("") - tags := p.parseTags(rawTags) + tags := p.parseTags(rawTags, cache.NewInternerContext(kint, "", &cache.SmallRetainer{})) assert.Nil(t, tags) } @@ -68,7 +71,8 @@ func TestUnsafeParseFloat(t *testing.T) { func TestUnsafeParseFloatList(t *testing.T) { cfg := fxutil.Test[config.Component](t, config.MockModule) - p := newParser(cfg, newFloat64ListPool()) + kint := cache.NewKeyedStringInternerForTest() + p := newParser(cfg, newFloat64ListPool(), kint) unsafeFloats, err := p.parseFloat64List([]byte("1.1234:21.5:13")) assert.NoError(t, err) assert.Len(t, unsafeFloats, 3) diff --git a/comp/dogstatsd/server/server.go b/comp/dogstatsd/server/server.go index 9e9aba7b454c5..397fe10a3f898 100644 --- a/comp/dogstatsd/server/server.go +++ b/comp/dogstatsd/server/server.go @@ -19,7 +19,6 @@ import ( "runtime/pprof" "strings" "sync" - "sync/atomic" "time" configComponent "github.com/DataDog/datadog-agent/comp/core/config" @@ -38,6 +37,7 @@ import ( "github.com/DataDog/datadog-agent/pkg/telemetry" "github.com/DataDog/datadog-agent/pkg/util" "github.com/DataDog/datadog-agent/pkg/util/hostname" + "go.uber.org/atomic" "go.uber.org/fx" ) @@ -148,7 +148,7 @@ type server struct { enrichConfig enrichConfig - nrEvents, nrServiceChecks, nrMetricSamples uint64 + nrEvents, nrServiceChecks, nrMetricSamples *atomic.Int64 interner *cache.KeyedInterner } @@ -157,40 +157,42 @@ type profileMetadata struct { Name string `json:"name"` Timestamp string `json:"timestamp"` Interval string `json:"interval"` - Events uint64 `json:"eventCount"` - ServiceChecks uint64 `json:"serviceCheckCount"` - MetricSamples uint64 `json:"metricSampleCount"` + Events int64 `json:"eventCount"` + ServiceChecks int64 `json:"serviceCheckCount"` + MetricSamples int64 `json:"metricSampleCount"` } func (s *server) dumpProfiles() { var interval time.Duration var prefix string var dest string - if configs := strings.Split(os.Getenv("DD_SELF_PROFILE_MEM"), ","); len(configs) != 3 { + configs := strings.Split(os.Getenv("DD_SELF_PROFILE_MEM"), ",") + if len(configs) != 3 { + return + } + + prefix = configs[0] + var err error + interval, err = time.ParseDuration(configs[1]) + if err != nil { + _, _ = fmt.Fprintf(os.Stderr, "Failed to parse duration %s: %s\n", configs[1], err) return - } else { - prefix = configs[0] - var err error - interval, err = time.ParseDuration(configs[1]) - if err != nil { - _, _ = fmt.Fprintf(os.Stderr, "Failed to parse duration %s: %s\n", configs[1], err) - return - } - if interval < 1*time.Minute { - _, _ = fmt.Fprintf(os.Stderr, "Parsed duration (%s) was too small, making it 1 minute\n", interval) - interval = time.Minute - } - dest = configs[2] } - var priorEvents, priorServiceChecks, priorMetricSamples uint64 = 0, 0, 0 + if interval < 1*time.Minute { + _, _ = fmt.Fprintf(os.Stderr, "Parsed duration (%s) was too small, making it 1 minute\n", interval) + interval = time.Minute + } + dest = configs[2] + + var priorEvents, priorServiceChecks, priorMetricSamples int64 = 0, 0, 0 for { time.Sleep(interval) timestamp := time.Now().String() metadataFname := path.Join(dest, fmt.Sprintf("%s-%s.json", prefix, timestamp)) profileFname := path.Join(dest, fmt.Sprintf("%s-%s.pprof", prefix, timestamp)) - nrEvents := atomic.LoadUint64(&s.nrEvents) - nrServiceChecks := atomic.LoadUint64(&s.nrServiceChecks) - nrMetricSamples := atomic.LoadUint64(&s.nrMetricSamples) + nrEvents := s.nrEvents.Load() + nrServiceChecks := s.nrServiceChecks.Load() + nrMetricSamples := s.nrMetricSamples.Load() metadata := profileMetadata{ Name: prefix, Timestamp: timestamp, @@ -272,7 +274,7 @@ func initTelemetry(cfg config.ConfigReader, logger logComponent.Component) { // TODO: (components) - remove once serverless is an FX app func NewServerlessServer() Component { return newServerCompat(config.Datadog, logComponent.NewTemporaryLoggerWithoutInit(), replay.NewServerlessTrafficCapture(), serverDebug.NewServerlessServerDebug(), true, - cache.NewKeyedStringInternerVals(20240, false)) + cache.NewKeyedStringInternerMemOnly(20240)) } // TODO: (components) - merge with newServerCompat once NewServerlessServer is removed @@ -381,7 +383,10 @@ func newServerCompat(cfg config.ConfigReader, log logComponent.Component, captur serverlessMode: serverless, originOptOutEnabled: cfg.GetBool("dogstatsd_origin_optout_enabled"), }, - interner: interner, + nrEvents: atomic.NewInt64(0), + nrServiceChecks: atomic.NewInt64(0), + nrMetricSamples: atomic.NewInt64(0), + interner: interner, } return s } @@ -678,27 +683,26 @@ func (s *server) parsePackets(batcher *batcher, parser *parser, packets []*packe switch messageType { case serviceCheckType: - serviceCheck, err := s.parseServiceCheckMessage(parser, message, packet.Origin, batcher) + serviceCheck, err := s.parseServiceCheckMessage(parser, message, packet.Origin, batcher.Retainer()) if err != nil { s.errLog("Dogstatsd: error parsing service check '%q': %s", message, err) continue } - atomic.AddUint64(&s.nrServiceChecks, 1) + s.nrServiceChecks.Add(1) batcher.appendServiceCheck(serviceCheck) case eventType: - event, err := s.parseEventMessage(parser, message, packet.Origin, batcher) + event, err := s.parseEventMessage(parser, message, packet.Origin, batcher.Retainer()) if err != nil { s.errLog("Dogstatsd: error parsing event '%q': %s", message, err) continue } - atomic.AddUint64(&s.nrEvents, 1) + s.nrEvents.Add(1) batcher.appendEvent(event) case metricSampleType: var err error samples = samples[0:0] - - samples, err = s.parseMetricMessage(samples, parser, message, packet.Origin, s.originTelemetry, batcher) + samples, err = s.parseMetricMessage(samples, parser, message, packet.Origin, s.originTelemetry) if err != nil { s.errLog("Dogstatsd: error parsing metric message '%q': %s", message, err) continue @@ -720,7 +724,7 @@ func (s *server) parsePackets(batcher *batcher, parser *parser, packets []*packe batcher.appendSample(*distSample) } } - atomic.AddUint64(&s.nrMetricSamples, uint64(len(samples))) + s.nrMetricSamples.Add(int64(len(samples))) } } @@ -775,14 +779,15 @@ func (s *server) getOriginCounter(origin string) (okCnt telemetry.SimpleCounter, // which will be slower when processing millions of samples. It could use a boolean returned by `parseMetricSample` which // is the first part aware of processing a late metric. Also, it may help us having a telemetry of a "late_metrics" type here // which we can't do today. -func (s *server) parseMetricMessage(metricSamples []metrics.MetricSample, parser *parser, message []byte, origin string, originTelemetry bool, retainer cache.InternRetainer) ([]metrics.MetricSample, error) { +func (s *server) parseMetricMessage(metricSamples []metrics.MetricSample, parser *parser, message []byte, origin string, originTelemetry bool) ([]metrics.MetricSample, error) { okCnt := tlmProcessedOk errorCnt := tlmProcessedError if origin != "" && originTelemetry { okCnt, errorCnt = s.getOriginCounter(origin) } - sample, err := parser.parseMetricSample(message, cache.NewInternerContext(s.interner, origin, retainer)) + retainer := cache.SmallRetainer{} + sample, err := parser.parseMetricSample(message, cache.NewInternerContext(s.interner, origin, &retainer)) if err != nil { dogstatsdMetricParseErrors.Add(1) errorCnt.Inc() @@ -793,7 +798,7 @@ func (s *server) parseMetricMessage(metricSamples []metrics.MetricSample, parser mapResult := s.mapper.Map(sample.name) if mapResult != nil { s.log.Tracef("Dogstatsd mapper: metric mapped from %q to %q with tags %v", sample.name, mapResult.Name, mapResult.Tags) - sample.name = mapResult.Name + sample.name = s.interner.LoadOrStoreString(mapResult.Name, origin, &retainer) sample.tags = append(sample.tags, mapResult.Tags...) } } @@ -815,6 +820,7 @@ func (s *server) parseMetricMessage(metricSamples []metrics.MetricSample, parser dogstatsdMetricPackets.Add(1) okCnt.Inc() } + metricSamples[len(metricSamples)-1].References.Import(&retainer) return metricSamples, nil } diff --git a/comp/dogstatsd/server/server_bench_test.go b/comp/dogstatsd/server/server_bench_test.go index bcc9b5a1cc641..8195c2d7968a5 100644 --- a/comp/dogstatsd/server/server_bench_test.go +++ b/comp/dogstatsd/server/server_bench_test.go @@ -8,6 +8,7 @@ package server import ( + "github.com/DataDog/datadog-agent/pkg/util/cache" "testing" "time" @@ -31,7 +32,7 @@ func mockDemultiplexerWithFlushInterval(config config.Component, log log.Compone opts.DontStartForwarders = true forwarder := forwarder.NewDefaultForwarder(config, log, forwarder.NewOptions(config, log, nil)) - demux := aggregator.InitAndStartAgentDemultiplexer(log, forwarder, opts, "hostname") + demux := aggregator.InitAndStartAgentDemultiplexer(log, forwarder, opts, "hostname", nil) return demux } @@ -70,7 +71,9 @@ func benchParsePackets(b *testing.B, rawPacket []byte) { b.RunParallel(func(pb *testing.PB) { batcher := newBatcher(demux.AgentDemultiplexer) - parser := newParser(deps.Config, newFloat64ListPool()) + kint := cache.NewKeyedStringInternerForTest() + + parser := newParser(deps.Config, newFloat64ListPool(), kint) packet := packets.Packet{ Contents: rawPacket, Origin: packets.NoOrigin, @@ -115,8 +118,8 @@ func BenchmarkPbarseMetricMessage(b *testing.B) { } }() defer close(done) - - parser := newParser(deps.Config, newFloat64ListPool()) + kint := cache.NewKeyedStringInternerForTest() + parser := newParser(deps.Config, newFloat64ListPool(), kint) message := []byte("daemon:666|h|@0.5|#sometag1:somevalue1,sometag2:somevalue2") b.RunParallel(func(pb *testing.PB) { @@ -170,7 +173,8 @@ func benchmarkMapperControl(b *testing.B, yaml string) { defer close(done) batcher := newBatcher(demux.AgentDemultiplexer) - parser := newParser(deps.Config, newFloat64ListPool()) + kint := cache.NewKeyedStringInternerForTest() + parser := newParser(deps.Config, newFloat64ListPool(), kint) samples := make([]metrics.MetricSample, 0, 512) for n := 0; n < b.N; n++ { diff --git a/comp/dogstatsd/server/server_test.go b/comp/dogstatsd/server/server_test.go index 7979c372368cb..6aaef39e35abf 100644 --- a/comp/dogstatsd/server/server_test.go +++ b/comp/dogstatsd/server/server_test.go @@ -9,6 +9,7 @@ package server import ( "fmt" + "github.com/DataDog/datadog-agent/pkg/util/cache" "net" "sort" "strings" @@ -35,9 +36,10 @@ import ( type serverDeps struct { fx.In - Server Component - Config configComponent.Component - Log log.Component + Server Component + Config configComponent.Component + Log log.Component + Interner *cache.KeyedInterner } func fulfillDeps(t testing.TB) serverDeps { @@ -54,6 +56,7 @@ func fulfillDepsWithConfigOverrideAndFeatures(t testing.TB, overrides map[string }), fx.Supply(Params{Serverless: false}), replay.MockModule, + cache.Module, Module, )) } @@ -71,6 +74,7 @@ func fulfillDepsWithConfigYaml(t testing.TB, yaml string) serverDeps { }), fx.Supply(Params{Serverless: false}), replay.MockModule, + cache.Module, Module, )) } @@ -676,8 +680,9 @@ func TestNoMappingsConfig(t *testing.T) { requireStart(t, s, demux) assert.Nil(t, s.mapper) + kint := cache.NewKeyedStringInternerForTest() - parser := newParser(deps.Config, newFloat64ListPool()) + parser := newParser(deps.Config, newFloat64ListPool(), kint) samples, err := s.parseMetricMessage(samples, parser, []byte("test.metric:666|g"), "", false) assert.NoError(t, err) assert.Len(t, samples, 1) @@ -789,9 +794,11 @@ dogstatsd_mapper_profiles: assert.Equal(t, deps.Config.Get("dogstatsd_mapper_cache_size"), scenario.expectedCacheSize, "Case `%s` failed. cache_size `%s` should be `%s`", scenario.name, deps.Config.Get("dogstatsd_mapper_cache_size"), scenario.expectedCacheSize) + kint := cache.NewKeyedStringInternerForTest() + var actualSamples []MetricSample for _, p := range scenario.packets { - parser := newParser(deps.Config, newFloat64ListPool()) + parser := newParser(deps.Config, newFloat64ListPool(), kint) samples, err := s.parseMetricMessage(samples, parser, []byte(p), "", false) assert.NoError(t, err, "Case `%s` failed. parseMetricMessage should not return error %v", err) for _, sample := range samples { @@ -866,7 +873,8 @@ func TestProcessedMetricsOrigin(t *testing.T) { assert.Len(s.cachedOriginCounters, 0, "this cache must be empty") assert.Len(s.cachedOrder, 0, "this cache list must be empty") - parser := newParser(deps.Config, newFloat64ListPool()) + kint := cache.NewKeyedStringInternerForTest() + parser := newParser(deps.Config, newFloat64ListPool(), kint) samples := []metrics.MetricSample{} samples, err := s.parseMetricMessage(samples, parser, []byte("test.metric:666|g"), "container_id://test_container", false) assert.NoError(err) @@ -939,8 +947,9 @@ func testContainerIDParsing(t *testing.T, cfg map[string]interface{}) { assert := assert.New(t) requireStart(t, s, mockDemultiplexer(deps.Config, deps.Log)) s.Stop() + kint := cache.NewKeyedStringInternerForTest() - parser := newParser(deps.Config, newFloat64ListPool()) + parser := newParser(deps.Config, newFloat64ListPool(), kint) parser.dsdOriginEnabled = true // Metric @@ -950,13 +959,13 @@ func testContainerIDParsing(t *testing.T, cfg map[string]interface{}) { assert.Equal("container_id://metric-container", metrics[0].OriginFromClient) // Event - event, err := s.parseEventMessage(parser, []byte("_e{10,10}:event title|test\\ntext|c:event-container"), "") + event, err := s.parseEventMessage(parser, []byte("_e{10,10}:event title|test\\ntext|c:event-container"), "", &cache.SmallRetainer{}) assert.NoError(err) assert.NotNil(event) assert.Equal("container_id://event-container", event.OriginFromClient) // Service check - serviceCheck, err := s.parseServiceCheckMessage(parser, []byte("_sc|service-check.name|0|c:service-check-container"), "") + serviceCheck, err := s.parseServiceCheckMessage(parser, []byte("_sc|service-check.name|0|c:service-check-container"), "", &cache.SmallRetainer{}) assert.NoError(err) assert.NotNil(serviceCheck) assert.Equal("container_id://service-check-container", serviceCheck.OriginFromClient) @@ -981,8 +990,8 @@ func testOriginOptout(t *testing.T, cfg map[string]interface{}, enabled bool) { requireStart(t, s, mockDemultiplexer(deps.Config, deps.Log)) s.Stop() - - parser := newParser(deps.Config, newFloat64ListPool()) + kint := cache.NewKeyedStringInternerForTest() + parser := newParser(deps.Config, newFloat64ListPool(), kint) parser.dsdOriginEnabled = true // Metric @@ -996,7 +1005,7 @@ func testOriginOptout(t *testing.T, cfg map[string]interface{}, enabled bool) { } // Event - event, err := s.parseEventMessage(parser, []byte("_e{10,10}:event title|test\\ntext|c:event-container|#dd.internal.card:none"), "") + event, err := s.parseEventMessage(parser, []byte("_e{10,10}:event title|test\\ntext|c:event-container|#dd.internal.card:none"), "", &cache.SmallRetainer{}) assert.NoError(err) assert.NotNil(event) if enabled { @@ -1006,7 +1015,7 @@ func testOriginOptout(t *testing.T, cfg map[string]interface{}, enabled bool) { } // Service check - serviceCheck, err := s.parseServiceCheckMessage(parser, []byte("_sc|service-check.name|0|c:service-check-container|#dd.internal.card:none"), "") + serviceCheck, err := s.parseServiceCheckMessage(parser, []byte("_sc|service-check.name|0|c:service-check-container|#dd.internal.card:none"), "", &cache.SmallRetainer{}) assert.NoError(err) assert.NotNil(serviceCheck) if enabled { diff --git a/pkg/aggregator/aggregator.go b/pkg/aggregator/aggregator.go index 65f706850eaa8..6731affc69d8a 100644 --- a/pkg/aggregator/aggregator.go +++ b/pkg/aggregator/aggregator.go @@ -267,6 +267,9 @@ func NewBufferedAggregator(s serializer.MetricSerializer, eventPlatformForwarder // Override the agentName if this Agent is configured to report as Heroku Dyno agentName = flavor.HerokuAgent } + if interner == nil { + interner = cache.NewKeyedStringInternerMemOnly(512) + } tagsStore := tags.NewStore(config.Datadog.GetBool("aggregator_use_tags_store"), "aggregator", interner) diff --git a/pkg/aggregator/aggregator_test.go b/pkg/aggregator/aggregator_test.go index 4ca0d8ff4166d..0da4c00e8db02 100644 --- a/pkg/aggregator/aggregator_test.go +++ b/pkg/aggregator/aggregator_test.go @@ -54,13 +54,25 @@ func initF() { opts.DontStartForwarders = true log := log.NewTemporaryLoggerWithoutInit() forwarder := defaultforwarder.NewDefaultForwarder(pkgconfig.Datadog, log, defaultforwarder.NewOptions(pkgconfig.Datadog, log, nil)) - demux := InitAndStartAgentDemultiplexer(log, forwarder, opts, defaultHostname) + demux := InitAndStartAgentDemultiplexer(log, forwarder, opts, defaultHostname, nil) demux.Aggregator().tlmContainerTagsEnabled = false // do not use a ContainerImpl recurrentSeries = metrics.Series{} tagsetTlm.reset() } +func normalize(s *metrics.Serie) *metrics.Serie { + s.References.Drop() + return s +} + +func normalizeMany(s metrics.Series) metrics.Series { + for i := range s { + s[i].References.Drop() + } + return s +} + func testNewFlushTrigger(start time.Time, waitForSerializer bool) flushTrigger { seriesSink := metrics.NewIterableSeries(func(se *metrics.Serie) {}, 1000, 1000) flushedSketches := make(metrics.SketchSeriesList, 0) @@ -152,7 +164,7 @@ func TestAddServiceCheckDefaultValues(t *testing.T) { // - s := &MockSerializerIterableSerie{} - agg := NewBufferedAggregator(s, nil, "resolved-hostname", DefaultFlushInterval) + agg := NewBufferedAggregator(s, nil, "resolved-hostname", DefaultFlushInterval, nil) agg.addServiceCheck(servicecheck.ServiceCheck{ // leave Host and Ts fields blank @@ -184,7 +196,7 @@ func TestAddEventDefaultValues(t *testing.T) { // - s := &MockSerializerIterableSerie{} - agg := NewBufferedAggregator(s, nil, "resolved-hostname", DefaultFlushInterval) + agg := NewBufferedAggregator(s, nil, "resolved-hostname", DefaultFlushInterval, nil) agg.addEvent(event.Event{ // only populate required fields @@ -233,7 +245,7 @@ func TestDefaultData(t *testing.T) { // - s := &MockSerializerIterableSerie{} - agg := NewBufferedAggregator(s, nil, "hostname", DefaultFlushInterval) + agg := NewBufferedAggregator(s, nil, "hostname", DefaultFlushInterval, nil) start := time.Now() // Check only the name for `datadog.agent.up` as the timestamp may not be the same. @@ -480,7 +492,7 @@ func TestRecurrentSeries(t *testing.T) { s.On("SendServiceChecks", agentUpMatcher).Return(nil).Times(1) demux.ForceFlushToSerializer(start, true) - require.EqualValues(t, expectedSeries, s.series) + require.EqualValues(t, expectedSeries, normalizeMany(s.series)) s.series = nil s.AssertNotCalled(t, "SendEvents") @@ -490,7 +502,7 @@ func TestRecurrentSeries(t *testing.T) { // same goes for the service check s.On("SendServiceChecks", agentUpMatcher).Return(nil).Times(1) demux.ForceFlushToSerializer(start, true) - require.EqualValues(t, expectedSeries, s.series) + require.EqualValues(t, expectedSeries, normalizeMany(s.series)) s.series = nil s.AssertNotCalled(t, "SendEvents") @@ -582,7 +594,7 @@ func TestTags(t *testing.T) { t.Run(tt.name, func(t *testing.T) { defer pkgconfig.Datadog.Set("basic_telemetry_add_container_tags", nil) pkgconfig.Datadog.Set("basic_telemetry_add_container_tags", tt.tlmContainerTagsEnabled) - agg := NewBufferedAggregator(nil, nil, tt.hostname, time.Second) + agg := NewBufferedAggregator(nil, nil, tt.hostname, time.Second, nil) agg.agentTags = tt.agentTags agg.globalTags = tt.globalTags assert.ElementsMatch(t, tt.want, agg.tags(tt.withVersion)) @@ -613,7 +625,8 @@ func TestTimeSamplerFlush(t *testing.T) { // MockSerializerIterableSerie overrides `SendIterableSeries` to avoid this issue. // It also overrides `SendSeries` for simplificy. type MockSerializerIterableSerie struct { - series []*metrics.Serie + //series []*metrics.Serie + series metrics.Series serializer.MockSerializer } @@ -688,7 +701,7 @@ func assertSeriesEqual(t *testing.T, series []*metrics.Serie, expectedSeries map sort.Slice(expected.Points, func(i int, j int) bool { return expected.Points[i].Ts < expected.Points[j].Ts }) - r.EqualValues(expected, serie) + r.EqualValues(normalize(expected), normalize(serie)) } r.Empty(expectedSeries) diff --git a/pkg/aggregator/check_sampler.go b/pkg/aggregator/check_sampler.go index fe451a893a669..69716303fda34 100644 --- a/pkg/aggregator/check_sampler.go +++ b/pkg/aggregator/check_sampler.go @@ -58,7 +58,7 @@ func (cs *CheckSampler) addSample(metricSample *metrics.MetricSample) { } } -func (cs *CheckSampler) newSketchSeries(ck ckey.ContextKey, points []metrics.SketchPoint) *metrics.SketchSeries { +func (cs *CheckSampler) newSketchSeries(ck ckey.ContextKey, points []metrics.SketchPoint, refs cache.InternRetainer) *metrics.SketchSeries { ctx, _ := cs.contextResolver.get(ck) ss := &metrics.SketchSeries{ // Interval: TODO: investigate @@ -70,6 +70,7 @@ func (cs *CheckSampler) newSketchSeries(ck ckey.ContextKey, points []metrics.Ske return cs.contextResolver.resolver.interner.LoadOrStoreString(s, cache.OriginCheckSampler, &ss.References) }) ss.Host = cs.contextResolver.resolver.interner.LoadOrStoreString(ctx.Host, cache.OriginCheckSampler, &ss.References) + ss.References.Import(refs) return ss } @@ -165,15 +166,20 @@ func (cs *CheckSampler) commitSeries(timestamp float64) { func (cs *CheckSampler) commitSketches(timestamp float64) { pointsByCtx := make(map[ckey.ContextKey][]metrics.SketchPoint) + refsByCtx := make(map[ckey.ContextKey]cache.SmallRetainer) - cs.sketchMap.flushBefore(int64(timestamp), func(ck ckey.ContextKey, p metrics.SketchPoint) { + cs.sketchMap.flushBefore(int64(timestamp), func(ck ckey.ContextKey, p metrics.SketchPoint, r cache.InternRetainer) { if p.Sketch == nil { return } pointsByCtx[ck] = append(pointsByCtx[ck], p) + retainer := refsByCtx[ck] + retainer.Import(r) + refsByCtx[ck] = retainer }) for ck, points := range pointsByCtx { - cs.sketches = append(cs.sketches, cs.newSketchSeries(ck, points)) + retainer := refsByCtx[ck] + cs.sketches = append(cs.sketches, cs.newSketchSeries(ck, points, &retainer)) } } diff --git a/pkg/aggregator/check_sampler_test.go b/pkg/aggregator/check_sampler_test.go index 5e4b4fc3e6871..49885d13f5dd4 100644 --- a/pkg/aggregator/check_sampler_test.go +++ b/pkg/aggregator/check_sampler_test.go @@ -33,7 +33,7 @@ func generateContextKey(sample metrics.MetricSampleContext) ckey.ContextKey { } func testCheckGaugeSampling(t *testing.T, store *tags.Store) { - checkSampler := newCheckSampler(1, true, 1*time.Second, store) + checkSampler := newCheckSampler(1, true, 1*time.Second, store, nil) mSample1 := metrics.MetricSample{ Name: "my.metric.name", @@ -95,7 +95,7 @@ func TestCheckGaugeSampling(t *testing.T) { } func testCheckRateSampling(t *testing.T, store *tags.Store) { - checkSampler := newCheckSampler(1, true, 1*time.Second, store) + checkSampler := newCheckSampler(1, true, 1*time.Second, store, nil) mSample1 := metrics.MetricSample{ Name: "my.metric.name", @@ -147,7 +147,7 @@ func TestCheckRateSampling(t *testing.T) { } func testHistogramCountSampling(t *testing.T, store *tags.Store) { - checkSampler := newCheckSampler(1, true, 1*time.Second, store) + checkSampler := newCheckSampler(1, true, 1*time.Second, store, nil) mSample1 := metrics.MetricSample{ Name: "my.metric.name", @@ -211,7 +211,7 @@ func TestHistogramCountSampling(t *testing.T) { } func testCheckHistogramBucketSampling(t *testing.T, store *tags.Store) { - checkSampler := newCheckSampler(1, true, 1*time.Second, store) + checkSampler := newCheckSampler(1, true, 1*time.Second, store, nil) bucket1 := &metrics.HistogramBucket{ Name: "my.histogram", @@ -287,7 +287,7 @@ func TestCheckHistogramBucketSampling(t *testing.T) { } func testCheckHistogramBucketDontFlushFirstValue(t *testing.T, store *tags.Store) { - checkSampler := newCheckSampler(1, true, 1*time.Second, store) + checkSampler := newCheckSampler(1, true, 1*time.Second, store, nil) bucket1 := &metrics.HistogramBucket{ Name: "my.histogram", @@ -342,7 +342,7 @@ func TestCheckHistogramBucketDontFlushFirstValue(t *testing.T) { } func testCheckHistogramBucketInfinityBucket(t *testing.T, store *tags.Store) { - checkSampler := newCheckSampler(1, true, 1*time.Second, store) + checkSampler := newCheckSampler(1, true, 1*time.Second, store, nil) bucket1 := &metrics.HistogramBucket{ Name: "my.histogram", @@ -376,7 +376,7 @@ func TestCheckHistogramBucketInfinityBucket(t *testing.T) { } func testCheckDistribution(t *testing.T, store *tags.Store) { - checkSampler := newCheckSampler(1, true, 1*time.Second, store) + checkSampler := newCheckSampler(1, true, 1*time.Second, store, nil) mSample1 := metrics.MetricSample{ Name: "my.metric.name", diff --git a/pkg/aggregator/context_resolver.go b/pkg/aggregator/context_resolver.go index a61bda3b25628..4c58074dbd45f 100644 --- a/pkg/aggregator/context_resolver.go +++ b/pkg/aggregator/context_resolver.go @@ -59,11 +59,14 @@ func (cr *contextResolver) generateContextKey(metricSampleContext metrics.Metric cache.CheckDefault(metricSampleContext.GetHost()), cr.taggerBuffer, cr.metricBuffer) } -func newContextResolver(cache *tags.Store, contextsLimiter *limiter.Limiter, tagsLimiter *tags_limiter.Limiter, interner *cache.KeyedInterner) *contextResolver { +func newContextResolver(store *tags.Store, contextsLimiter *limiter.Limiter, tagsLimiter *tags_limiter.Limiter, interner *cache.KeyedInterner) *contextResolver { + if interner == nil { + interner = cache.NewKeyedStringInternerMemOnly(512) + } return &contextResolver{ contextsByKey: make(map[ckey.ContextKey]*Context), countsByMtype: make([]uint64, metrics.NumMetricTypes), - tagsCache: cache, + tagsCache: store, keyGenerator: ckey.NewKeyGenerator(), taggerBuffer: tagset.NewHashingTagsAccumulator(), metricBuffer: tagset.NewHashingTagsAccumulator(), @@ -122,9 +125,8 @@ func (cr *contextResolver) referenceContext(contextKey ckey.ContextKey) (cache.S ctx.metricTags.Retainer.CopyTo(&refs) ctx.references.CopyTo(&refs) return refs, true - } else { - return refs, false } + return refs, false } func (cr *contextResolver) tryAdd(taggerKey ckey.TagsKey) bool { @@ -255,7 +257,9 @@ func (cr *timestampContextResolver) get(key ckey.ContextKey) (*Context, bool) { // expireContexts cleans up the contexts that haven't been tracked since the given timestamp // and returns the associated contextKeys. // keep can be used to retain contexts longer than their natural expiration time based on some condition. -func (cr *timestampContextResolver) expireContexts(expireTimestamp float64, keep func(ckey.ContextKey) bool) { +func (cr *timestampContextResolver) expireContexts(expireTimestamp float64, + + keep func(ckey.ContextKey) bool) { for contextKey, lastSeen := range cr.lastSeenByKey { if lastSeen < expireTimestamp && (keep == nil || !keep(contextKey)) { delete(cr.lastSeenByKey, contextKey) diff --git a/pkg/aggregator/context_resolver_test.go b/pkg/aggregator/context_resolver_test.go index 0d5df5015640a..294e56e47e47f 100644 --- a/pkg/aggregator/context_resolver_test.go +++ b/pkg/aggregator/context_resolver_test.go @@ -8,6 +8,7 @@ package aggregator import ( + "github.com/DataDog/datadog-agent/pkg/util/cache" // stdlib "testing" @@ -25,13 +26,13 @@ import ( // Helper functions to run tests and benchmarks for context resolver, time and check samplers. func testWithTagsStore(t *testing.T, test func(*testing.T, *tags.Store)) { - t.Run("useStore=true", func(t *testing.T) { test(t, tags.NewStore(true, "test")) }) - t.Run("useStore=false", func(t *testing.T) { test(t, tags.NewStore(false, "test")) }) + t.Run("useStore=true", func(t *testing.T) { test(t, tags.NewStore(true, "test", cache.NewKeyedStringInternerMemOnly(128))) }) + t.Run("useStore=false", func(t *testing.T) { test(t, tags.NewStore(false, "test", cache.NewKeyedStringInternerMemOnly(128))) }) } func benchWithTagsStore(t *testing.B, test func(*testing.B, *tags.Store)) { - t.Run("useStore=true", func(t *testing.B) { test(t, tags.NewStore(true, "test")) }) - t.Run("useStore=false", func(t *testing.B) { test(t, tags.NewStore(false, "test")) }) + t.Run("useStore=true", func(t *testing.B) { test(t, tags.NewStore(true, "test", cache.NewKeyedStringInternerMemOnly(128))) }) + t.Run("useStore=false", func(t *testing.B) { test(t, tags.NewStore(false, "test", cache.NewKeyedStringInternerMemOnly(128))) }) } func assertContext(t *testing.T, cx *Context, name string, tags []string, host string) { @@ -78,7 +79,7 @@ func testTrackContext(t *testing.T, store *tags.Store) { SampleRate: 1, } - contextResolver := newContextResolver(store, nil, nil) + contextResolver := newContextResolver(store, nil, nil, nil) // Track the 2 contexts contextKey1, _ := contextResolver.trackContext(&mSample1) @@ -123,7 +124,7 @@ func testExpireContexts(t *testing.T, store *tags.Store) { Tags: []string{"foo", "bar", "baz"}, SampleRate: 1, } - contextResolver := newTimestampContextResolver(store, nil, nil) + contextResolver := newTimestampContextResolver(store, nil, nil, nil) // Track the 2 contexts contextKey1, _ := contextResolver.trackContext(&mSample1, 4) @@ -165,7 +166,7 @@ func testExpireContextsWithKeep(t *testing.T, store *tags.Store) { Tags: []string{"foo", "bar", "baz"}, SampleRate: 1, } - contextResolver := newTimestampContextResolver(store, nil, nil) + contextResolver := newTimestampContextResolver(store, nil, nil, nil) // Track the 2 contexts contextKey1, _ := contextResolver.trackContext(&mSample1, 4) @@ -242,7 +243,7 @@ func TestCountBasedExpireContexts(t *testing.T) { } func testTagDeduplication(t *testing.T, store *tags.Store) { - resolver := newContextResolver(store, nil, nil) + resolver := newContextResolver(store, nil, nil, nil) ckey, _ := resolver.trackContext(&metrics.MetricSample{ Name: "foo", @@ -279,8 +280,16 @@ func (s *mockSample) GetTags(tb, mb tagset.TagsAccumulator) { mb.Append(s.metricTags...) } +func clearSink(s mockSink) mockSink { + for i := range s { + s[i].References.Drop() + } + return s +} + func TestOriginTelemetry(t *testing.T) { - r := newContextResolver(tags.NewStore(true, "test"), nil, nil) + kint := cache.NewKeyedStringInternerMemOnly(512) + r := newContextResolver(tags.NewStore(true, "test", kint), nil, nil, kint) r.trackContext(&mockSample{"foo", []string{"foo"}, []string{"ook"}}) r.trackContext(&mockSample{"foo", []string{"foo"}, []string{"eek"}}) r.trackContext(&mockSample{"foo", []string{"bar"}, []string{"ook"}}) @@ -290,7 +299,7 @@ func TestOriginTelemetry(t *testing.T) { ts := 1672835152.0 r.sendOriginTelemetry(ts, &sink, "test", []string{"test"}) - assert.ElementsMatch(t, sink, []*metrics.Serie{{ + assert.ElementsMatch(t, clearSink(sink), []*metrics.Serie{{ Name: "datadog.agent.aggregator.dogstatsd_contexts_by_origin", Host: "test", Tags: tagset.NewCompositeTags([]string{"test"}, []string{"foo"}), @@ -314,7 +323,8 @@ func TestOriginTelemetry(t *testing.T) { func TestLimiterTelemetry(t *testing.T) { l := limiter.New(2, "pod", []string{"pod", "srv"}) tl := tags_limiter.New(4) - r := newContextResolver(tags.NewStore(true, "test"), l, tl) + kint := cache.NewKeyedStringInternerMemOnly(512) + r := newContextResolver(tags.NewStore(true, "test", kint), l, tl, kint) r.trackContext(&mockSample{"foo", []string{"pod:foo", "srv:foo"}, []string{"pod:bar"}}) r.trackContext(&mockSample{"foo", []string{"pod:foo", "srv:foo"}, []string{"srv:bar"}}) r.trackContext(&mockSample{"bar", []string{"pod:foo", "srv:foo"}, []string{"srv:bar"}}) @@ -326,7 +336,7 @@ func TestLimiterTelemetry(t *testing.T) { ts := 1672835152.0 r.sendLimiterTelemetry(ts, &sink, "test", []string{"test"}) - assert.Subset(t, sink, []*metrics.Serie{{ + assert.Subset(t, clearSink(sink), []*metrics.Serie{{ Name: "datadog.agent.aggregator.dogstatsd_context_limiter.current", Host: "test", Tags: tagset.NewCompositeTags([]string{"test"}, []string{"pod:foo", "srv:foo"}), @@ -372,9 +382,10 @@ func TestLimiterTelemetry(t *testing.T) { } func TestTimestampContextResolverLimit(t *testing.T) { - store := tags.NewStore(true, "") + kint := cache.NewKeyedStringInternerMemOnly(512) + store := tags.NewStore(true, "", kint) limiter := limiter.New(1, "pod", []string{}) - r := newTimestampContextResolver(store, limiter, nil) + r := newTimestampContextResolver(store, limiter, nil, kint) r.trackContext(&mockSample{"foo", []string{"pod:foo", "srv:foo"}, []string{"pod:bar"}}, 42) r.trackContext(&mockSample{"foo", []string{"pod:foo", "srv:foo"}, []string{"srv:bar"}}, 42) diff --git a/pkg/aggregator/demultiplexer.go b/pkg/aggregator/demultiplexer.go index f8fc866721b47..7d9ac8af3548e 100644 --- a/pkg/aggregator/demultiplexer.go +++ b/pkg/aggregator/demultiplexer.go @@ -136,12 +136,14 @@ type flushTrigger struct { sketchesSink metrics.SketchesSink seriesSink metrics.SerieSink + retainer cache.InternRetainer } func createIterableMetrics( flushAndSerializeInParallel FlushAndSerializeInParallel, serializer serializer.MetricSerializer, - retainer cache.InternRetainer, + seriesRetainer cache.InternRetainer, + sketchRetainer cache.InternRetainer, logPayloads bool, isServerless bool, ) (*metrics.IterableSeries, *metrics.IterableSketches) { @@ -153,8 +155,8 @@ func createIterableMetrics( if logPayloads { log.Debugf("Flushing serie: %s", se) } - if retainer != nil { - retainer.Import(&se.References) + if seriesRetainer != nil { + seriesRetainer.Import(&se.References) } tagsetTlm.updateHugeSerieTelemetry(se) @@ -169,8 +171,8 @@ func createIterableMetrics( if isServerless { log.DebugfServerless("Sending sketches payload : %s", sketch.String()) } - if retainer != nil { - retainer.Import(&sketch.References) + if sketchRetainer != nil { + sketchRetainer.Import(&sketch.References) } tagsetTlm.updateHugeSketchesTelemetry(sketch) }, flushAndSerializeInParallel.BufferSize, flushAndSerializeInParallel.ChannelSize) diff --git a/pkg/aggregator/demultiplexer_agent.go b/pkg/aggregator/demultiplexer_agent.go index 6ef29e44489f1..22f3d857ad92a 100644 --- a/pkg/aggregator/demultiplexer_agent.go +++ b/pkg/aggregator/demultiplexer_agent.go @@ -141,10 +141,10 @@ type AggregatorTestDeps struct { fx.In Log log.Component SharedForwarder defaultforwarder.Component - Interner *cache.KeyedInterner } -func InitAndStartAgentDemultiplexerForTest(deps AggregatorTestDeps, options AgentDemultiplexerOptions, hostname string, interner *cache.KeyedInterner) *AgentDemultiplexer { +func InitAndStartAgentDemultiplexerForTest(deps AggregatorTestDeps, options AgentDemultiplexerOptions, hostname string) *AgentDemultiplexer { + interner := cache.NewKeyedStringInternerMemOnly(16384) return InitAndStartAgentDemultiplexer(deps.Log, deps.SharedForwarder, options, hostname, interner) } @@ -196,7 +196,7 @@ func initAgentDemultiplexer(log log.Component, sharedForwarder forwarder.Forward if interner == nil { // Test cases don't require an interner to work, don't require that they create one for us. - interner = cache.NewKeyedStringInternerVals(16384, false) + interner = cache.NewKeyedStringInternerMemOnly(16384) } for i := 0; i < statsdPipelinesCount; i++ { @@ -364,11 +364,9 @@ func (d *AgentDemultiplexer) flushLoop() { if trigger.blockChan != nil { trigger.blockChan <- struct{}{} } - d.LogRetentions() // automatic flush sequence case t := <-flushTicker: d.flushToSerializer(t, false) - d.LogRetentions() } } } @@ -479,16 +477,24 @@ func (d *AgentDemultiplexer) flushToSerializer(start time.Time, waitForSerialize logPayloads := config.Datadog.GetBool("log_payloads") const retTag = "IterableMetrics" - iterableRetentions := d.retentions[retTag] - if iterableRetentions == nil { - iterableRetentions = cache.NewRetainerBlock() - d.retentions[retTag] = iterableRetentions + const statsTag = "StatsdWorkers" + const checkTag = "CheckSamplerWorkers" + const seriesTag = "SeriesIterables" + const sketchTag = "SketchIterables" + allTags := []string{retTag, statsTag, checkTag, seriesTag, sketchTag} + + for _, t := range allTags { + if _, ok := d.retentions[t]; !ok { + d.retentions[t] = cache.NewRetainerBlock() + } } + for _, retainer := range d.retentions { retainer.ReleaseAll() } - series, sketches := createIterableMetrics(d.aggregator.flushAndSerializeInParallel, d.sharedSerializer, iterableRetentions, logPayloads, false) + series, sketches := createIterableMetrics(d.aggregator.flushAndSerializeInParallel, d.sharedSerializer, d.retentions[seriesTag], + d.retentions[sketchTag], logPayloads, false) metrics.Serialize( series, @@ -506,6 +512,7 @@ func (d *AgentDemultiplexer) flushToSerializer(start time.Time, waitForSerialize }, sketchesSink: sketchesSink, seriesSink: seriesSink, + retainer: d.retentions[statsTag], } worker.flushChan <- t @@ -524,6 +531,7 @@ func (d *AgentDemultiplexer) flushToSerializer(start time.Time, waitForSerialize }, sketchesSink: sketchesSink, seriesSink: seriesSink, + retainer: d.retentions[checkTag], } d.aggregator.flushChan <- t @@ -628,7 +636,10 @@ func (d *AgentDemultiplexer) GetMetricSamplePool() *metrics.MetricSamplePool { return d.statsd.metricSamplePool } +// TakeRetentions will Import() all retentions from its parameter and save them under its tag. func (d *AgentDemultiplexer) TakeRetentions(retentions cache.InternRetainer, tag string) { + d.m.Lock() + defer d.m.Unlock() if ret, ok := d.retentions[tag]; !ok { ret = cache.NewRetainerBlock() d.retentions[tag] = ret @@ -637,10 +648,3 @@ func (d *AgentDemultiplexer) TakeRetentions(retentions cache.InternRetainer, tag ret.Import(retentions) } } - -func (d *AgentDemultiplexer) LogRetentions() { - d.log.Infof("cache.Retainer: Cache Retainers:") - for tag, retainer := range d.retentions { - d.log.Infof("cache.Retainer: %s: %s", tag, retainer.Summarize()) - } -} diff --git a/pkg/aggregator/demultiplexer_agent_test.go b/pkg/aggregator/demultiplexer_agent_test.go index 0592227c998eb..c68f256f51c50 100644 --- a/pkg/aggregator/demultiplexer_agent_test.go +++ b/pkg/aggregator/demultiplexer_agent_test.go @@ -54,7 +54,7 @@ func TestDemuxNoAggOptionDisabled(t *testing.T) { opts := demuxTestOptions() log := fxutil.Test[log.Component](t, log.MockModule) - demux := initAgentDemultiplexer(log, NewForwarderTest(log), opts, "") + demux := initAgentDemultiplexer(log, NewForwarderTest(log), opts, "", nil) batch := testDemuxSamples(t) @@ -74,7 +74,7 @@ func TestDemuxNoAggOptionEnabled(t *testing.T) { mockSerializer := &MockSerializerIterableSerie{} opts.EnableNoAggregationPipeline = true log := fxutil.Test[log.Component](t, log.MockModule) - demux := initAgentDemultiplexer(log, NewForwarderTest(log), opts, "") + demux := initAgentDemultiplexer(log, NewForwarderTest(log), opts, "", nil) demux.statsd.noAggStreamWorker.serializer = mockSerializer // the no agg pipeline will use our mocked serializer go demux.Run() diff --git a/pkg/aggregator/demultiplexer_serverless.go b/pkg/aggregator/demultiplexer_serverless.go index f624d3b9af0ed..e907ae9033376 100644 --- a/pkg/aggregator/demultiplexer_serverless.go +++ b/pkg/aggregator/demultiplexer_serverless.go @@ -48,7 +48,7 @@ func InitAndStartServerlessDemultiplexer(domainResolvers map[string]resolver.Dom tagsStore := tags.NewStore(config.Datadog.GetBool("aggregator_use_tags_store"), "timesampler", interner) statsdSampler := NewTimeSampler(TimeSamplerID(0), bucketSize, tagsStore, nil, nil, "", - cache.NewKeyedStringInternerVals(128, false)) + cache.NewKeyedStringInternerMemOnly(128)) flushAndSerializeInParallel := NewFlushAndSerializeInParallel(config.Datadog) statsdWorker := newTimeSamplerWorker(statsdSampler, DefaultFlushInterval, bufferSize, metricSamplePool, flushAndSerializeInParallel, tagsStore) @@ -106,7 +106,7 @@ func (d *ServerlessDemultiplexer) ForceFlushToSerializer(start time.Time, waitFo defer d.flushLock.Unlock() logPayloads := config.Datadog.GetBool("log_payloads") - series, sketches := createIterableMetrics(d.flushAndSerializeInParallel, d.serializer, nil, logPayloads, true) + series, sketches := createIterableMetrics(d.flushAndSerializeInParallel, d.serializer, nil, nil, logPayloads, true) metrics.Serialize( series, @@ -172,8 +172,6 @@ func (d *ServerlessDemultiplexer) GetMetricSamplePool() *metrics.MetricSamplePoo return d.metricSamplePool } +// TakeRetentions will Import retentions from its argument and release sometime later. func (d *ServerlessDemultiplexer) TakeRetentions(retentions cache.InternRetainer, tag string) { - // TODO: Figure out the lifetime of serverless and see if we actually want to do reference - // counting at all for it. Alternatively, never release and let the agent die. - panic("not implemented") } diff --git a/pkg/aggregator/demultiplexer_test.go b/pkg/aggregator/demultiplexer_test.go index 9efd883dbbafe..c4c91de679cef 100644 --- a/pkg/aggregator/demultiplexer_test.go +++ b/pkg/aggregator/demultiplexer_test.go @@ -183,7 +183,7 @@ func TestDemuxFlushAggregatorToSerializer(t *testing.T) { opts := demuxTestOptions() opts.FlushInterval = time.Hour deps := fxutil.Test[AggregatorTestDeps](t, defaultforwarder.MockModule, config.MockModule, log.MockModule) - demux := initAgentDemultiplexer(deps.Log, deps.SharedForwarder, opts, "") + demux := initAgentDemultiplexer(deps.Log, deps.SharedForwarder, opts, "", nil) demux.Aggregator().tlmContainerTagsEnabled = false require.NotNil(demux) require.NotNil(demux.aggregator) diff --git a/pkg/aggregator/internal/limiter/limiter_test.go b/pkg/aggregator/internal/limiter/limiter_test.go index 1943aaf25cb24..1bb14cf88cb93 100644 --- a/pkg/aggregator/internal/limiter/limiter_test.go +++ b/pkg/aggregator/internal/limiter/limiter_test.go @@ -6,12 +6,14 @@ package limiter import ( + "github.com/DataDog/datadog-agent/pkg/util/cache" "testing" "github.com/stretchr/testify/assert" ) func TestLimiter(t *testing.T) { + interner := cache.NewKeyedStringInternerMemOnly(512) l := New(1, "pod", []string{"srv"}) // check that: @@ -23,50 +25,54 @@ func TestLimiter(t *testing.T) { a.Equal(l.telemetryTagNames, []string{"srv:", "pod:"}) - a.True(l.Track([]string{"srv:foo", "cid:1", "pod", "pod:foo"})) - a.False(l.Track([]string{"srv:foo", "cid:2", "pod", "pod:foo"})) + a.True(l.Track([]string{"srv:foo", "cid:1", "pod", "pod:foo"}, interner)) + a.False(l.Track([]string{"srv:foo", "cid:2", "pod", "pod:foo"}, interner)) - a.True(l.Track([]string{"srv:foo", "cid:3", "pod", "pod:bar"})) - a.False(l.Track([]string{"srv:foo", "cid:4", "pod", "pod:bar"})) + a.True(l.Track([]string{"srv:foo", "cid:3", "pod", "pod:bar"}, interner)) + a.False(l.Track([]string{"srv:foo", "cid:4", "pod", "pod:bar"}, interner)) - a.True(l.Track([]string{"srv:foo", "cid:5", "pod"})) - a.False(l.Track([]string{"srv:foo", "cid:6", "pod"})) - a.False(l.Track([]string{})) + a.True(l.Track([]string{"srv:foo", "cid:5", "pod"}, interner)) + a.False(l.Track([]string{"srv:foo", "cid:6", "pod"}, interner)) + a.False(l.Track([]string{}, interner)) l.Remove([]string{}) - a.True(l.Track([]string{})) + a.True(l.Track([]string{}, interner)) l.Remove([]string{"srv:bar", "pod:foo"}) - a.True(l.Track([]string{"srv:bar", "pod:foo"})) + a.True(l.Track([]string{"srv:bar", "pod:foo"}, interner)) + + usage := l.usage["pod:foo"] + usage.retentions.Drop() a.Equal(&entry{ current: 1, rejected: 0, telemetryTags: []string{"srv:bar", "pod:foo"}, - }, l.usage["pod:foo"]) + }, usage) l.Remove([]string{"pod:foo"}) a.Nil(l.usage["pod:foo"]) } func TestGlobal(t *testing.T) { + interner := cache.NewKeyedStringInternerMemOnly(512) l := NewGlobal(2, 1, "pod", []string{}) a := assert.New(t) - a.True(l.Track([]string{"pod:foo"})) - a.True(l.Track([]string{"pod:foo"})) - a.False(l.Track([]string{"pod:foo"})) - a.False(l.Track([]string{"pod:bar"})) // would exceed global limit + a.True(l.Track([]string{"pod:foo"}, interner)) + a.True(l.Track([]string{"pod:foo"}, interner)) + a.False(l.Track([]string{"pod:foo"}, interner)) + a.False(l.Track([]string{"pod:bar"}, interner)) // would exceed global limit l.Remove([]string{"pod:foo"}) - a.False(l.Track([]string{"pod:foo"})) // would exceed per-origin limit + a.False(l.Track([]string{"pod:foo"}, interner)) // would exceed per-origin limit - a.True(l.Track([]string{"pod:bar"})) - a.False(l.Track([]string{"pod:bar"})) // would exceed per-origin limit + a.True(l.Track([]string{"pod:bar"}, interner)) + a.False(l.Track([]string{"pod:bar"}, interner)) // would exceed per-origin limit l.Remove([]string{"pod:bar"}) // removes origin entry, limit is 2 again - a.True(l.Track([]string{"pod:foo"})) + a.True(l.Track([]string{"pod:foo"}, interner)) // check for division by zero l.Remove([]string{"pod:foo"}) @@ -75,26 +81,27 @@ func TestGlobal(t *testing.T) { } func TestExpire(t *testing.T) { + interner := cache.NewKeyedStringInternerMemOnly(512) l := NewGlobal(2, 1, "pod", []string{}) a := assert.New(t) foo := []string{"pod:foo"} bar := []string{"pod:bar"} - a.True(l.Track(foo)) - a.True(l.Track(foo)) - a.False(l.Track(bar)) // rejected, but allocates limit to bar + a.True(l.Track(foo, interner)) + a.True(l.Track(foo, interner)) + a.False(l.Track(bar, interner)) // rejected, but allocates limit to bar l.ExpireEntries() l.Remove(foo) // maxAge 1 means limit remains reserved for 1 tick after initial sample - a.False(l.Track(foo)) + a.False(l.Track(foo, interner)) a.Len(l.usage, 2) l.ExpireEntries() a.Len(l.usage, 1) l.Remove([]string{"pod:foo"}) - a.True(l.Track([]string{"pod:foo"})) + a.True(l.Track([]string{"pod:foo"}, interner)) } diff --git a/pkg/aggregator/internal/tags/store.go b/pkg/aggregator/internal/tags/store.go index e6ef5b4b071c7..ae2aec000b6c1 100644 --- a/pkg/aggregator/internal/tags/store.go +++ b/pkg/aggregator/internal/tags/store.go @@ -17,8 +17,6 @@ import ( "github.com/DataDog/datadog-agent/pkg/telemetry" ) -const OriginTagStore = "!ContextResolver" - // Entry is used to keep track of tag slices shared by the contexts. type Entry struct { // refs is the refcount of this entity. If this value is zero, then the @@ -79,7 +77,7 @@ func makeEntry(tagsBuffer *tagset.HashingTagsAccumulator, interner *cache.KeyedI } for n, t := range tagsBuffer.Copy() { - result.tags[n] = interner.LoadOrStoreString(t, OriginTagStore, &result.Retainer) + result.tags[n] = interner.LoadOrStoreString(t, cache.OriginContextResolver, &result.Retainer) } return result } diff --git a/pkg/aggregator/internal/tags/store_test.go b/pkg/aggregator/internal/tags/store_test.go index 2b33909e8149a..37b296db22655 100644 --- a/pkg/aggregator/internal/tags/store_test.go +++ b/pkg/aggregator/internal/tags/store_test.go @@ -6,6 +6,7 @@ package tags import ( + "github.com/DataDog/datadog-agent/pkg/util/cache" "testing" "github.com/DataDog/datadog-agent/pkg/aggregator/ckey" @@ -15,7 +16,9 @@ import ( ) func TestStore(t *testing.T) { - c := NewStore(true, "test") + interner := cache.NewKeyedStringInternerMemOnly(512) + + c := NewStore(true, "test", interner) t1 := tagset.NewHashingTagsAccumulatorWithTags([]string{"1"}) t2 := tagset.NewHashingTagsAccumulatorWithTags([]string{"2"}) @@ -84,7 +87,8 @@ func TestStore(t *testing.T) { } func TestStoreDisabled(t *testing.T) { - c := NewStore(false, "test") + interner := cache.NewKeyedStringInternerMemOnly(512) + c := NewStore(false, "test", interner) t1 := tagset.NewHashingTagsAccumulatorWithTags([]string{"1"}) t2 := tagset.NewHashingTagsAccumulatorWithTags([]string{"2"}) @@ -119,7 +123,8 @@ func TestStoreDisabled(t *testing.T) { } func BenchmarkRefCounting(b *testing.B) { - st := NewStore(true, "foo") + interner := cache.NewKeyedStringInternerMemOnly(512) + st := NewStore(true, "foo", interner) tagsBuffer := tagset.NewHashingTagsAccumulator() // Entries are only removed in Shrink, which isn't called in this diff --git a/pkg/aggregator/no_aggregation_stream_worker.go b/pkg/aggregator/no_aggregation_stream_worker.go index 57c85469feb73..7a392c768bc36 100644 --- a/pkg/aggregator/no_aggregation_stream_worker.go +++ b/pkg/aggregator/no_aggregation_stream_worker.go @@ -141,7 +141,7 @@ func (w *noAggregationStreamWorker) run() { ticker := time.NewTicker(noAggWorkerStreamCheckFrequency) defer ticker.Stop() logPayloads := config.Datadog.GetBool("log_payloads") - w.seriesSink, w.sketchesSink = createIterableMetrics(w.flushConfig, w.serializer, nil, logPayloads, false) + w.seriesSink, w.sketchesSink = createIterableMetrics(w.flushConfig, w.serializer, nil, nil, logPayloads, false) stopped := false var stopBlockChan chan struct{} @@ -242,7 +242,7 @@ func (w *noAggregationStreamWorker) run() { break } - w.seriesSink, w.sketchesSink = createIterableMetrics(w.flushConfig, w.serializer, nil, logPayloads, false) + w.seriesSink, w.sketchesSink = createIterableMetrics(w.flushConfig, w.serializer, nil, nil, logPayloads, false) } if stopBlockChan != nil { diff --git a/pkg/aggregator/sender_test.go b/pkg/aggregator/sender_test.go index 7ba09d84c8d1c..5fd1469606afb 100644 --- a/pkg/aggregator/sender_test.go +++ b/pkg/aggregator/sender_test.go @@ -47,7 +47,7 @@ func initSender(id checkid.ID, defaultHostname string) (s senderWithChans) { func testDemux(log log.Component) *AgentDemultiplexer { opts := DefaultAgentDemultiplexerOptions() opts.DontStartForwarders = true - demux := initAgentDemultiplexer(log, NewForwarderTest(log), opts, defaultHostname) + demux := initAgentDemultiplexer(log, NewForwarderTest(log), opts, defaultHostname, nil) return demux } diff --git a/pkg/aggregator/sketch_map.go b/pkg/aggregator/sketch_map.go index 1d4cfc0589268..baaa6f6ec856c 100644 --- a/pkg/aggregator/sketch_map.go +++ b/pkg/aggregator/sketch_map.go @@ -70,15 +70,18 @@ func (m sketchMap) getOrCreate(ts int64, ck ckey.ContextKey, refs cache.InternRe } // Keep references for this context's dependencies. - retainer := m[ts][ck].refs - retainer.Import(refs) + entry := m[ts][ck] + if refs != nil { + entry.refs.Import(refs) + } + m[ts][ck] = entry return s.agent } // flushBefore calls f for every sketch inserted before beforeTs, removing flushed sketches // from the map. -func (m sketchMap) flushBefore(beforeTs int64, f func(ckey.ContextKey, metrics.SketchPoint)) { +func (m sketchMap) flushBefore(beforeTs int64, f func(ckey.ContextKey, metrics.SketchPoint, cache.InternRetainer)) { for ts, byCtx := range m { if ts >= beforeTs { continue @@ -88,8 +91,7 @@ func (m sketchMap) flushBefore(beforeTs int64, f func(ckey.ContextKey, metrics.S f(ck, metrics.SketchPoint{ Sketch: as.agent.Finish(), Ts: ts, - }) - as.refs.ReleaseAll() + }, &as.refs) } delete(m, ts) diff --git a/pkg/aggregator/sketch_map_test.go b/pkg/aggregator/sketch_map_test.go index 83425dc110ebc..bdd95d7abc162 100644 --- a/pkg/aggregator/sketch_map_test.go +++ b/pkg/aggregator/sketch_map_test.go @@ -28,8 +28,8 @@ func TestInsert(t *testing.T) { SampleRate: 1, } - sketchMap.insert(1, generateContextKey(&mSample1), 1, 1) + sketchMap.insert(1, generateContextKey(&mSample1), 1, 1, nil) assert.Equal(t, 1, sketchMap.Len()) - sketchMap.insert(2, generateContextKey(&mSample1), 2, 1) + sketchMap.insert(2, generateContextKey(&mSample1), 2, 1, nil) assert.Equal(t, 2, sketchMap.Len()) } diff --git a/pkg/aggregator/time_sampler.go b/pkg/aggregator/time_sampler.go index 6c495879b60fc..f343a40b6c54c 100644 --- a/pkg/aggregator/time_sampler.go +++ b/pkg/aggregator/time_sampler.go @@ -41,7 +41,10 @@ type TimeSampler struct { // since we start running more than one with the demultiplexer introduction id TimeSamplerID - hostname string + // Accumulate the current retentions up to the current + // flush, then flush out the prior retentions from there. + retentions, priorRetentions cache.SmallRetainer + hostname string } // NewTimeSampler returns a newly initialized TimeSampler @@ -91,9 +94,15 @@ func (s *TimeSampler) sample(metricSample *metrics.MetricSample, timestamp float switch metricSample.Mtype { case metrics.DistributionType: + // Add references to the context for this distribution, and take the references for the + // metric samples. refs, _ := s.contextResolver.resolver.referenceContext(contextKey) + refs.Import(&metricSample.References) s.sketchMap.insert(bucketStart, contextKey, metricSample.Value, metricSample.SampleRate, &refs) + default: + // Save the references for the next flush. + s.retentions.Import(&metricSample.References) // If it's a new bucket, initialize it bucketMetrics, ok := s.metricsByTimestamp[bucketStart] if !ok { @@ -111,19 +120,20 @@ func (s *TimeSampler) sample(metricSample *metrics.MetricSample, timestamp float } } } -func (s *TimeSampler) newSketchSeries(ck ckey.ContextKey, points []metrics.SketchPoint) *metrics.SketchSeries { + +func (s *TimeSampler) newSketchSeries(ck ckey.ContextKey, points []metrics.SketchPoint, refs cache.InternRetainer) *metrics.SketchSeries { ctx, _ := s.contextResolver.get(ck) ss := &metrics.SketchSeries{ Interval: s.interval, Points: points, ContextKey: ck, } - ss.Name = s.interner.LoadOrStoreString(ctx.Name, cache.OriginTimeSampler+".Name", &ss.References) + ss.Name = ctx.Name // s.interner.LoadOrStoreString(ctx.Name, cache.OriginTimeSampler+".Name", &ss.References) ss.Tags = ctx.Tags().Apply(func(tag string) string { return s.interner.LoadOrStoreString(tag, cache.OriginTimeSampler+".Tags", &ss.References) }) ss.Host = s.interner.LoadOrStoreString(ctx.Host, cache.OriginTimeSampler+".Host", &ss.References) - + ss.References.Import(refs) return ss } @@ -215,19 +225,24 @@ func (s *TimeSampler) dedupSerieBySerieSignature( func (s *TimeSampler) flushSketches(cutoffTime int64, sketchesSink metrics.SketchesSink) { pointsByCtx := make(map[ckey.ContextKey][]metrics.SketchPoint) + refsByCtx := make(map[ckey.ContextKey]cache.SmallRetainer) - s.sketchMap.flushBefore(cutoffTime, func(ck ckey.ContextKey, p metrics.SketchPoint) { + s.sketchMap.flushBefore(cutoffTime, func(ck ckey.ContextKey, p metrics.SketchPoint, r cache.InternRetainer) { if p.Sketch == nil { return } pointsByCtx[ck] = append(pointsByCtx[ck], p) + retainer := refsByCtx[ck] + retainer.Import(r) + refsByCtx[ck] = retainer }) for ck, points := range pointsByCtx { - sketchesSink.Append(s.newSketchSeries(ck, points)) + retainer := refsByCtx[ck] + sketchesSink.Append(s.newSketchSeries(ck, points, &retainer)) } } -func (s *TimeSampler) flush(timestamp float64, series metrics.SerieSink, sketches metrics.SketchesSink) { +func (s *TimeSampler) flush(timestamp float64, series metrics.SerieSink, sketches metrics.SketchesSink, retainer cache.InternRetainer) { // Compute a limit timestamp cutoffTime := s.calculateBucketStart(timestamp) @@ -253,6 +268,11 @@ func (s *TimeSampler) flush(timestamp float64, series metrics.SerieSink, sketche tlmDogstatsdContextsByMtype.Set(float64(count), mtype) } + // Return the prior cycle's retentions. + if retainer != nil { + retainer.Import(&s.priorRetentions) + } + s.priorRetentions.Import(&s.retentions) s.sendTelemetry(timestamp, series) } diff --git a/pkg/aggregator/time_sampler_test.go b/pkg/aggregator/time_sampler_test.go index 5fc0efa723dff..72ec0082ad754 100644 --- a/pkg/aggregator/time_sampler_test.go +++ b/pkg/aggregator/time_sampler_test.go @@ -9,6 +9,7 @@ package aggregator import ( "fmt" + "github.com/DataDog/datadog-agent/pkg/util/cache" "math" "sort" "testing" @@ -35,7 +36,8 @@ func generateSerieContextKey(serie *metrics.Serie) ckey.ContextKey { } func testTimeSampler() *TimeSampler { - sampler := NewTimeSampler(TimeSamplerID(0), 10, tags.NewStore(false, "test"), nil, nil, "host") + kint := cache.NewKeyedStringInternerMemOnly(512) + sampler := NewTimeSampler(TimeSamplerID(0), 10, tags.NewStore(false, "test", kint), nil, nil, "host", kint) return sampler } @@ -516,7 +518,7 @@ func TestBucketSamplingWithSketchAndSeries(t *testing.T) { } func benchmarkTimeSampler(b *testing.B, store *tags.Store) { - sampler := NewTimeSampler(TimeSamplerID(0), 10, store, nil, nil, "host") + sampler := NewTimeSampler(TimeSamplerID(0), 10, store, nil, nil, "host", nil) sample := metrics.MetricSample{ Name: "my.metric.name", @@ -552,11 +554,13 @@ func BenchmarkTimeSamplerWithLimiter(b *testing.B) { Timestamp: 12345.0, } + kint := cache.NewKeyedStringInternerMemOnly(512) + for limit := range []int{0, 1, 2, 3} { - store := tags.NewStore(false, "test") + store := tags.NewStore(false, "test", kint) limiter := limiter.New(limit, "pod", []string{"pod"}) tagsLimiter := tags_limiter.New(5) - sampler := NewTimeSampler(TimeSamplerID(0), 10, store, limiter, tagsLimiter, "host") + sampler := NewTimeSampler(TimeSamplerID(0), 10, store, limiter, tagsLimiter, "host", nil) b.Run(fmt.Sprintf("limit=%d", limit), func(b *testing.B) { for n := 0; n < b.N; n++ { @@ -571,6 +575,6 @@ func flushSerie(sampler *TimeSampler, timestamp float64) (metrics.Series, metric var series metrics.Series var sketches metrics.SketchSeriesList - sampler.flush(timestamp, &series, &sketches) + sampler.flush(timestamp, &series, &sketches, &cache.SmallRetainer{}) return series, sketches } diff --git a/pkg/aggregator/time_sampler_worker.go b/pkg/aggregator/time_sampler_worker.go index 84c0cfd601b68..6fb18557b3a98 100644 --- a/pkg/aggregator/time_sampler_worker.go +++ b/pkg/aggregator/time_sampler_worker.go @@ -91,6 +91,6 @@ func (w *timeSamplerWorker) stop() { } func (w *timeSamplerWorker) triggerFlush(trigger flushTrigger) { - w.sampler.flush(float64(trigger.time.Unix()), trigger.seriesSink, trigger.sketchesSink) + w.sampler.flush(float64(trigger.time.Unix()), trigger.seriesSink, trigger.sketchesSink, trigger.retainer) trigger.blockChan <- struct{}{} } diff --git a/pkg/collector/corechecks/snmp/internal/checkconfig/config_test.go b/pkg/collector/corechecks/snmp/internal/checkconfig/config_test.go index 53f2bc294dab4..e43468beff7cd 100644 --- a/pkg/collector/corechecks/snmp/internal/checkconfig/config_test.go +++ b/pkg/collector/corechecks/snmp/internal/checkconfig/config_test.go @@ -21,7 +21,7 @@ import ( func TestConfigurations(t *testing.T) { SetConfdPathAndCleanProfiles() - aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour) + aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour, nil) // language=yaml rawInstanceConfig := []byte(` @@ -320,7 +320,7 @@ profiles: func TestInlineProfileConfiguration(t *testing.T) { SetConfdPathAndCleanProfiles() - aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour) + aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour, nil) // language=yaml rawInstanceConfig := []byte(` diff --git a/pkg/collector/corechecks/snmp/profile_metadata_test.go b/pkg/collector/corechecks/snmp/profile_metadata_test.go index acf714c4a3484..7b93f5bbc7b7d 100644 --- a/pkg/collector/corechecks/snmp/profile_metadata_test.go +++ b/pkg/collector/corechecks/snmp/profile_metadata_test.go @@ -31,7 +31,7 @@ import ( func TestProfileMetadata_f5(t *testing.T) { timeNow = common.MockTimeNow - aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour) + aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour, nil) invalidPath, _ := filepath.Abs(filepath.Join("internal", "test", "metadata.d")) config.Datadog.Set("confd_path", invalidPath) diff --git a/pkg/collector/corechecks/snmp/snmp_test.go b/pkg/collector/corechecks/snmp/snmp_test.go index a7d7c189bbba0..84f713593eac5 100644 --- a/pkg/collector/corechecks/snmp/snmp_test.go +++ b/pkg/collector/corechecks/snmp/snmp_test.go @@ -65,7 +65,7 @@ func Test_Run_simpleCase(t *testing.T) { return sess, nil } chk := Check{sessionFactory: sessionFactory} - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) // language=yaml rawInstanceConfig := []byte(` @@ -345,7 +345,7 @@ func Test_Run_customIfSpeed(t *testing.T) { } chk := Check{sessionFactory: sessionFactory} - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) // language=yaml rawInstanceConfig := []byte(` @@ -552,7 +552,7 @@ func TestProfile(t *testing.T) { timeNow = common.MockTimeNow deps := createDeps(t) - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) checkconfig.SetConfdPathAndCleanProfiles() @@ -1202,7 +1202,7 @@ community_string: public namespace: '%s' `, tt.name)) deps := createDeps(t) - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) err := chk.Configure(senderManager, integration.FakeConfigHash, rawInstanceConfig, []byte(``), "test") assert.Nil(t, err) @@ -1283,7 +1283,7 @@ func TestReportDeviceMetadataEvenOnProfileError(t *testing.T) { timeNow = common.MockTimeNow deps := createDeps(t) - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) checkconfig.SetConfdPathAndCleanProfiles() sess := session.CreateMockSession() @@ -1588,7 +1588,7 @@ tags: func TestReportDeviceMetadataWithFetchError(t *testing.T) { timeNow = common.MockTimeNow deps := createDeps(t) - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) checkconfig.SetConfdPathAndCleanProfiles() @@ -1702,7 +1702,7 @@ func TestDiscovery(t *testing.T) { return sess, nil } chk := Check{sessionFactory: sessionFactory} - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) // language=yaml rawInstanceConfig := []byte(` @@ -2042,7 +2042,7 @@ func TestDiscovery_CheckError(t *testing.T) { return sess, nil } chk := Check{sessionFactory: sessionFactory, workerRunDeviceCheckErrors: atomic.NewUint64(0)} - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) // language=yaml rawInstanceConfig := []byte(` @@ -2120,7 +2120,7 @@ func TestDeviceIDAsHostname(t *testing.T) { chk := Check{sessionFactory: sessionFactory} coreconfig.Datadog.Set("hostname", "test-hostname") coreconfig.Datadog.Set("tags", []string{"agent_tag1:val1", "agent_tag2:val2"}) - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) // language=yaml rawInstanceConfig := []byte(` @@ -2311,7 +2311,7 @@ func TestDiscoveryDeviceIDAsHostname(t *testing.T) { chk := Check{sessionFactory: sessionFactory} coreconfig.Datadog.Set("hostname", "my-hostname") - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) // language=yaml rawInstanceConfig := []byte(` @@ -2515,7 +2515,7 @@ func TestCheckCancel(t *testing.T) { } chk := Check{sessionFactory: sessionFactory} - senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "") + senderManager := aggregator.InitAndStartAgentDemultiplexer(deps.Log, deps.Forwarder, demuxOpts(), "", nil) // language=yaml rawInstanceConfig := []byte(` diff --git a/pkg/collector/corechecks/snmp/topology_integration_test.go b/pkg/collector/corechecks/snmp/topology_integration_test.go index c7c17b2f6e52b..8f130cea42524 100644 --- a/pkg/collector/corechecks/snmp/topology_integration_test.go +++ b/pkg/collector/corechecks/snmp/topology_integration_test.go @@ -31,7 +31,7 @@ import ( func TestTopologyPayload_LLDP(t *testing.T) { timeNow = common.MockTimeNow - aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour) + aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour, nil) invalidPath, _ := filepath.Abs(filepath.Join("internal", "test", "metadata.d")) config.Datadog.Set("confd_path", invalidPath) @@ -729,7 +729,7 @@ profiles: func TestTopologyPayload_CDP(t *testing.T) { timeNow = common.MockTimeNow - aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour) + aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour, nil) invalidPath, _ := filepath.Abs(filepath.Join("internal", "test", "metadata.d")) config.Datadog.Set("confd_path", invalidPath) @@ -1418,7 +1418,7 @@ profiles: // we have different data for LLDP and CDP to test that we're only using LLDP to build the links func TestTopologyPayload_LLDP_CDP(t *testing.T) { timeNow = common.MockTimeNow - aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour) + aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour, nil) invalidPath, _ := filepath.Abs(filepath.Join("internal", "test", "metadata.d")) config.Datadog.Set("confd_path", invalidPath) diff --git a/pkg/collector/corechecks/systemd/systemd_test.go b/pkg/collector/corechecks/systemd/systemd_test.go index ca905226e29fa..21da88907ffb7 100644 --- a/pkg/collector/corechecks/systemd/systemd_test.go +++ b/pkg/collector/corechecks/systemd/systemd_test.go @@ -1035,7 +1035,7 @@ unit_names: func TestCheckID(t *testing.T) { check1 := systemdFactory() check2 := systemdFactory() - aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour) + aggregator.NewBufferedAggregator(nil, nil, "", 1*time.Hour, nil) // language=yaml rawInstanceConfig1 := []byte(` diff --git a/pkg/config/config.go b/pkg/config/config.go index 2a5a0bc73fc47..2fa07687976ec 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -537,8 +537,13 @@ func InitConfig(config Config) { config.BindEnvAndSetDefault("dogstatsd_tags", []string{}) config.BindEnvAndSetDefault("dogstatsd_mapper_cache_size", 1000) config.BindEnvAndSetDefault("dogstatsd_string_interner_size", 4096) - // Unmap old interner pages. - config.BindEnvAndSetDefault("dogstatsd_string_interner_preserve_mmap", false) + // Whether to unmap old interner pages. Generally you do unless debugging something. + config.BindEnvAndSetDefault("dogstatsd_string_interner_mmap_preserve", false) + config.BindEnvAndSetDefault("dogstatsd_string_interner_tmpdir", os.TempDir()) + config.BindEnvAndSetDefault("dogstatsd_string_interner_mmap_enable", false) + config.BindEnvAndSetDefault("dogstatsd_string_interner_mmap_minsizekb", 65536) + config.BindEnvAndSetDefault("dogstatsd_string_interner_origin_max_strings", 64) + // Enable check for Entity-ID presence when enriching Dogstatsd metrics with tags config.BindEnvAndSetDefault("dogstatsd_entity_id_precedence", false) // Sends Dogstatsd parse errors to the Debug level instead of the Error level diff --git a/pkg/metrics/iterable_series_test.go b/pkg/metrics/iterable_series_test.go index 396c2c69a94e4..9b5ec712ce401 100644 --- a/pkg/metrics/iterable_series_test.go +++ b/pkg/metrics/iterable_series_test.go @@ -27,7 +27,7 @@ func TestIterableSeries(t *testing.T) { for serieSource.MoveNext() { names = append(names, serieSource.Current().Name) } - }, func(_ SketchesSource) {}, nil) + }, func(_ SketchesSource) {}) r := require.New(t) r.Len(names, 3) @@ -74,7 +74,7 @@ func BenchmarkIterableSeries(b *testing.B) { func(seriesSource SerieSource) { for seriesSource.MoveNext() { } - }, func(_ SketchesSource) {}, nil) + }, func(_ SketchesSource) {}) }) } } @@ -95,7 +95,7 @@ func TestIterableSeriesSeveralValues(t *testing.T) { for serieSource.MoveNext() { series = append(series, serieSource.Current()) } - }, func(_ SketchesSource) {}, nil) + }, func(_ SketchesSource) {}) r := require.New(t) r.Len(series, len(expected)) diff --git a/pkg/metrics/metric_sample.go b/pkg/metrics/metric_sample.go index 45f1cea3be768..5ceffdfd731fa 100644 --- a/pkg/metrics/metric_sample.go +++ b/pkg/metrics/metric_sample.go @@ -8,6 +8,7 @@ package metrics import ( "github.com/DataDog/datadog-agent/pkg/tagger" "github.com/DataDog/datadog-agent/pkg/tagset" + "github.com/DataDog/datadog-agent/pkg/util/cache" ) // MetricType is the representation of an aggregator metric type @@ -101,6 +102,7 @@ type MetricSample struct { NoIndex bool Origin string Source MetricSource + References cache.SmallRetainer } // Implement the MetricSampleContext interface diff --git a/pkg/metrics/series.go b/pkg/metrics/series.go index 78252db425ba4..d531e64662b9b 100644 --- a/pkg/metrics/series.go +++ b/pkg/metrics/series.go @@ -79,14 +79,15 @@ func (a APIMetricType) SeriesAPIV2Enum() int32 { } } -func (s *Serie) RetainCopy(ctx *cache.KeyedInterner, origin string) *Serie { - s.Name = ctx.LoadOrStoreString(s.Name, origin, &s.References) - s.Tags.Apply(func(t string) string { - return ctx.LoadOrStoreString(t, origin, &s.References) +// RetainCopy replaces each string with a Retained copy of them, keeping references. +func (serie *Serie) RetainCopy(ctx *cache.KeyedInterner, origin string) *Serie { + serie.Name = ctx.LoadOrStoreString(serie.Name, origin, &serie.References) + serie.Tags.Apply(func(t string) string { + return ctx.LoadOrStoreString(t, origin, &serie.References) }) - s.Host = ctx.LoadOrStoreString(s.Host, origin, &s.References) - s.Device = ctx.LoadOrStoreString(s.Device, origin, &s.References) - return s + serie.Host = ctx.LoadOrStoreString(serie.Host, origin, &serie.References) + serie.Device = ctx.LoadOrStoreString(serie.Device, origin, &serie.References) + return serie } // UnmarshalJSON is a custom unmarshaller for Point (used for testing) diff --git a/pkg/tagset/composite_tags.go b/pkg/tagset/composite_tags.go index b0064d1f19b05..8a5c6a6c48d7c 100644 --- a/pkg/tagset/composite_tags.go +++ b/pkg/tagset/composite_tags.go @@ -123,6 +123,7 @@ func (t CompositeTags) Find(callback func(tag string) bool) bool { return false } +// Apply a function to all tags. func (t CompositeTags) Apply(callback func(tag string) string) CompositeTags { for n, tag := range t.tags1 { if cache.Check(tag) { diff --git a/pkg/util/cache/component.go b/pkg/util/cache/component.go new file mode 100644 index 0000000000000..39b849cd98ce9 --- /dev/null +++ b/pkg/util/cache/component.go @@ -0,0 +1,13 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + +package cache + +import ( + "go.uber.org/fx" +) + +// Module for testing components that need KeyedStringInterners. +var Module = fx.Module("cache", fx.Provide(NewKeyedStringInterner)) diff --git a/pkg/util/cache/intern.go b/pkg/util/cache/intern.go index 0fcf5d1c0e2b2..11a6a46f53f77 100644 --- a/pkg/util/cache/intern.go +++ b/pkg/util/cache/intern.go @@ -10,10 +10,10 @@ import ( cconfig "github.com/DataDog/datadog-agent/comp/core/config" "github.com/DataDog/datadog-agent/pkg/util/log" lru "github.com/hashicorp/golang-lru" + "go.uber.org/atomic" "math" "os" "sync" - "sync/atomic" "time" "unsafe" ) @@ -29,31 +29,46 @@ const initialInternerSize = 64 const backingBytesPerInCoreEntry = 4096 const growthFactor = 1.5 -// TODO: Move this to a config arg. -const defaultTmpPath = "/tmp" +// noFileCache indicates that no mmap should be created. +const noFileCache = "" + +// OriginTimeSampler marks allocations to the Time Sampler. const OriginTimeSampler = "!Timesampler" + +// OriginContextResolver marks allocations to the Context Resolver. const OriginContextResolver = "!ContextResolver" + +// OriginCheckSampler marks allocations to the check sampler. const OriginCheckSampler = "!CheckSampler" + +// OriginBufferedAggregator marks allocations to the BufferedAggregator. const OriginBufferedAggregator = "!BufferedAggregator" + +// OriginContextLimiter marks allocations to the Context Limiter. const OriginContextLimiter = "!OriginContextLimiter" type stringInterner struct { - cache lruStringCache - fileBacking *mmap_hash // if this is nil, the string interner acts as a regular LRU string cache. - maxSize int - refcount int32 - refcountLock sync.Mutex + cache lruStringCache + fileBacking *mmapHash // if this is nil, the string interner acts as a regular LRU string cache. + maxStringCount int + refcount int32 + refcountLock sync.Mutex } +// Name of the interner - based on its origin. func (i *stringInterner) Name() string { if i.fileBacking != nil { - return i.fileBacking.Name - } else { - return "unbacked interner" + return i.fileBacking.Name() } + return "unbacked interner" +} + +// bytesPerEntry returns the number +func bytesPerEntry(maxStringCount int) int64 { + return int64(maxStringCount * backingBytesPerInCoreEntry) } -func newStringInterner(origin string, maxSize int, tmpPath string, closeOnRelease bool) *stringInterner { +func newStringInterner(origin string, maxStringCount int, tmpPath string, closeOnRelease bool) *stringInterner { // First version: a basic mmap'd file. Nothing fancy. Later: refcount system for // each interner. When the mmap goes to zero, unmap it WHEN we have a newer // version there. @@ -61,21 +76,22 @@ func newStringInterner(origin string, maxSize int, tmpPath string, closeOnReleas // new larger mmap and start interning from there (up to some quota we later worry about). // Old mmap gets removed when all strings referencing it get finalized. New strings won't be // created. - var backing *mmap_hash = nil - var err error = nil - if tmpPath != "" { - backing, err = newMmapHash(origin, int64(maxSize*backingBytesPerInCoreEntry), tmpPath, closeOnRelease) + var backing *mmapHash + var err error + if tmpPath != noFileCache { + backing, err = newMmapHash(origin, bytesPerEntry(maxStringCount), tmpPath, closeOnRelease) if err != nil { + log.Errorf("Failed to create MMAP hash file: %v", err) return nil } } i := &stringInterner{ - cache: newLruStringCache(maxSize, false), - fileBacking: backing, - maxSize: maxSize, - refcount: 1, + cache: newLruStringCache(maxStringCount, false), + fileBacking: backing, + maxStringCount: maxStringCount, + refcount: 1, } - log.Warnf("Created new String interner %p with mmap hash %p with max size %d", i, backing, maxSize) + log.Debugf("Created new String interner %p with mmap hash %p with max size %d", i, backing, maxStringCount) return i } @@ -89,7 +105,7 @@ func (i *stringInterner) loadOrStore(key []byte) string { // Empty key return "" } - if i.maxSize == 0 { + if i.maxStringCount == 0 { // Dead interner. release() has already been called, or it was broken on // construction. return "" @@ -104,15 +120,14 @@ func (i *stringInterner) loadOrStore(key []byte) string { // on the heap. Let GC take care of it normally. The string cache // already de-duplicates for us. return string(key) - } else { - return "" } - } else { - return s + // Return the empty string here, and "loadOrStore's" caller will + // resize the interner in response. + return "" } - } else { - return string(key) + return s } + return string(key) }) return result @@ -123,18 +138,26 @@ func (i *stringInterner) used() int64 { return used } +// Retain some references +func (i *stringInterner) Retain(n int32) { + i.refcountLock.Lock() + defer i.refcountLock.Unlock() + i.refcount += n +} + +// Release some references func (i *stringInterner) Release(n int32) { i.refcountLock.Lock() defer i.refcountLock.Unlock() if i.refcount < 1 { - log.Infof("Dead stringInterner begin released! refcount=%d", i.refcount) + log.Warnf("Dead stringInterner being released! refcount=%d", i.refcount) return } if i.refcount > 0 && i.refcount-n < 1 { - log.Infof("Finalizing backing, refcount=%d, n=%d", i.refcount, n) + log.Debugf("Finalizing backing, refcount=%d, n=%d", i.refcount, n) i.fileBacking.finalize() i.cache = newLruStringCache(0, false) - i.maxSize = 0 + i.maxStringCount = 0 } i.refcount -= n } @@ -143,29 +166,40 @@ func (i *stringInterner) retain() { i.refcountLock.Lock() defer i.refcountLock.Unlock() if i.refcount < 1 { - log.Errorf("Dead interner being re-retained!") + log.Error("Dead interner being re-retained!") } - i.refcount += 1 + i.refcount++ } +// KeyedInterner has an origin-keyed set of interners. type KeyedInterner struct { interners *lru.Cache maxQuota int closeOnRelease bool tmpPath string lastReport time.Time + minFileSize int64 + maxPerInterner int lock sync.Mutex } // NewKeyedStringInterner creates a Keyed String Interner with a max per-origin quota of maxQuota func NewKeyedStringInterner(cfg cconfig.Component) *KeyedInterner { - closeOnRelease := !cfg.GetBool("dogstatsd_string_interner_preserve_mmap") stringInternerCacheSize := cfg.GetInt("dogstatsd_string_interner_size") - - return NewKeyedStringInternerVals(stringInternerCacheSize, closeOnRelease) + enableMMap := cfg.GetBool("dogstatsd_string_interner_mmap_enable") + + if enableMMap { + closeOnRelease := !cfg.GetBool("dogstatsd_string_interner_mmap_preserve") + tempPath := cfg.GetString("dogstatsd_string_interner_tmpdir") + minSizeKb := cfg.GetInt("dogstatsd_string_interner_mmap_minsizekb") + maxStringsPerInterner := cfg.GetInt("dogstatsd_string_interner_origin_max_strings") + return NewKeyedStringInternerVals(stringInternerCacheSize, closeOnRelease, tempPath, minSizeKb, maxStringsPerInterner) + } + return NewKeyedStringInternerMemOnly(stringInternerCacheSize) } -func NewKeyedStringInternerVals(stringInternerCacheSize int, closeOnRelease bool) *KeyedInterner { +// NewKeyedStringInternerVals takes args explicitly for initialization +func NewKeyedStringInternerVals(stringInternerCacheSize int, closeOnRelease bool, tempPath string, minFileKb, maxStringsPerInterner int) *KeyedInterner { cache, err := lru.NewWithEvict(stringInternerCacheSize, func(_, internerUntyped interface{}) { interner := internerUntyped.(*stringInterner) interner.Release(1) @@ -177,7 +211,9 @@ func NewKeyedStringInternerVals(stringInternerCacheSize int, closeOnRelease bool interners: cache, maxQuota: -1, lastReport: time.Now(), - tmpPath: defaultTmpPath, + tmpPath: tempPath, + minFileSize: int64(minFileKb * 1024), + maxPerInterner: maxStringsPerInterner, closeOnRelease: closeOnRelease, } } @@ -196,18 +232,32 @@ func NewKeyedStringInternerMemOnly(stringInternerCacheSize int) *KeyedInterner { maxQuota: -1, lastReport: time.Now(), tmpPath: "", + maxPerInterner: initialInternerSize, closeOnRelease: false, } } -var s_globalQueryCount uint64 = 0 +// NewKeyedStringInternerForTest is a memory-only cache with a small default size. Useful for +// most tests. +func NewKeyedStringInternerForTest() *KeyedInterner { + return NewKeyedStringInternerMemOnly(512) +} + +var sGlobalQueryCount = atomic.NewInt64(0) +var sFailedInternalCount = atomic.NewInt64(0) +// LoadOrStoreString interns a string for an origin func (i *KeyedInterner) LoadOrStoreString(s string, origin string, retainer InternRetainer) string { - return i.LoadOrStore(unsafe.Slice(unsafe.StringData(s), len(s)), origin, retainer) + if Check(s) { + return i.LoadOrStore(unsafe.Slice(unsafe.StringData(s), len(s)), origin, retainer) + } + sFailedInternalCount.Add(1) + return "" } +// LoadOrStore interns a byte-array to a string, for an origin func (i *KeyedInterner) LoadOrStore(key []byte, origin string, retainer InternRetainer) string { - atomic.AddUint64(&s_globalQueryCount, 1) + sGlobalQueryCount.Add(1) keyLen := len(key) // Avoid locking for dumb stuff. if keyLen == 0 { @@ -219,6 +269,14 @@ func (i *KeyedInterner) LoadOrStore(key []byte, origin string, retainer InternRe return i.loadOrStore(key, origin, retainer) } +func (i *KeyedInterner) makeInterner(origin string, stringMaxCount int) *stringInterner { + if bytesPerEntry(stringMaxCount) >= i.minFileSize { + return newStringInterner(origin, stringMaxCount, i.tmpPath, i.closeOnRelease) + } + // No file cache until we get bigger. + return newStringInterner(origin, stringMaxCount, noFileCache, i.closeOnRelease) +} + func (i *KeyedInterner) loadOrStore(key []byte, origin string, retainer InternRetainer) string { // The mutex usage is pretty rudimentary. Upon profiling, have a look at better synchronization. // E.g., lock-free LRU. @@ -226,12 +284,13 @@ func (i *KeyedInterner) loadOrStore(key []byte, origin string, retainer InternRe defer i.lock.Unlock() if i.lastReport.Before(time.Now().Add(-1 * time.Minute)) { - log.Infof("*** INTERNER *** Keyed Interner has %d interners. closeOnRelease=%v, Total Query Count: %v", i.interners.Len(), i.closeOnRelease, - atomic.LoadUint64(&s_globalQueryCount)) + log.Debugf("*** INTERNER *** Keyed Interner has %d interners. closeOnRelease=%v, Total Query Count: %v, Total Failures: %v", i.interners.Len(), i.closeOnRelease, + sGlobalQueryCount.Load(), sFailedInternalCount.Load()) Report() i.lastReport = time.Now() } - var interner *stringInterner = nil + + var interner *stringInterner if i.interners.Contains(origin) { internerUntyped, _ := i.interners.Get(origin) interner = internerUntyped.(*stringInterner) @@ -242,45 +301,45 @@ func (i *KeyedInterner) loadOrStore(key []byte, origin string, retainer InternRe return "" } } else { - interner = newStringInterner(origin, initialInternerSize, i.tmpPath, i.closeOnRelease) - _ = log.Warnf("Creating string interner at %p for origin %v", interner, origin) + interner = i.makeInterner(origin, i.maxPerInterner) + log.Debugf("Creating string interner at %p for origin %v", interner, origin) i.interners.Add(origin, interner) } - if ret := interner.loadOrStore(key); ret == "" { + ret := interner.loadOrStore(key) + if ret == "" { // The only way the interner won't return a string is if it's full. Make a new bigger one and // start using that. We'll eventually migrate all the in-use strings to this from this container. - log.Infof("Failed interning string. Adding new interner for key %v, length %v", string(key), len(key)) - Report() - replacementInterner := newStringInterner(origin, int(math.Ceil(float64(interner.maxSize)*growthFactor)), i.tmpPath, i.closeOnRelease) - + log.Debugf("Failed interning string. Adding new interner for key %v, length %v", string(key), len(key)) + replacementInterner := i.makeInterner(origin, int(math.Ceil(float64(interner.maxStringCount)*growthFactor))) + if replacementInterner == nil { + // We couldn't intern the string nor create a new interner, so just heap allocate. newStringInterner + // will log errors when it fails like this. + return string(key) + } // We have one retention on the interner upon creation, to keep it available for ongoing use. // Release that now. i.interners.Add(origin, replacementInterner) retainer.Reference(replacementInterner) replacementInterner.retain() - log.Infof("Releasing old interner. Prior: %p -> New: %p", interner, replacementInterner) + log.Debugf("Releasing old interner. Prior: %p -> New: %p", interner, replacementInterner) interner.Release(1) return replacementInterner.loadOrStore(key) - } else { - interner.retain() - retainer.Reference(interner) - return ret } + interner.retain() + retainer.Reference(interner) + return ret } -func (i *KeyedInterner) Release(retainer InternRetainer) { - retainer.ReleaseAllWith(func(obj Refcounted, count int32) { - obj.Release(count) - }) -} - +// InternerContext saves all the arguments to LoadOrStore to avoid passing separately through function +// calls. type InternerContext struct { interner *KeyedInterner origin string retainer InternRetainer } +// NewInternerContext creates a new one, binding the args for future calls to LoadOrStore func NewInternerContext(interner *KeyedInterner, origin string, retainer InternRetainer) InternerContext { return InternerContext{ interner: interner, @@ -289,24 +348,13 @@ func NewInternerContext(interner *KeyedInterner, origin string, retainer InternR } } +// UseStringBytes calls LoadOrStore on the saved interner with the saved arguments. Add +// the given suffix to the origin. func (i *InternerContext) UseStringBytes(s []byte, suffix string) string { // TODO: Assume that the string is almost certainly already intern'd. // TODO: Validate here. - //s = CheckDefault(s) if i == nil { return string(s) - } else { - return i.interner.LoadOrStore(s, i.origin+suffix, i.retainer) } + return i.interner.LoadOrStore(s, i.origin+suffix, i.retainer) } - -/* -func (i *InternerContext) UseString(s string, suffix string) string { - s = CheckDefault(s) - // TODO: Assume that the string is almost certainly already intern'd. - if i == nil { - return s - } else { - return i.interner.LoadOrStore(unsafe.Slice(unsafe.StringData(s), len(s)), i.origin+suffix, i.retainer) - } -}*/ diff --git a/pkg/util/cache/intern_test.go b/pkg/util/cache/intern_test.go index e6309ae8c4947..bbafdbcabe37e 100644 --- a/pkg/util/cache/intern_test.go +++ b/pkg/util/cache/intern_test.go @@ -15,6 +15,21 @@ type localRetainer struct { retentions map[Refcounted]int32 } +func (r *localRetainer) ReferenceN(obj Refcounted, n int32) { + //TODO implement me + panic("implement me") +} + +func (r *localRetainer) CopyTo(dest InternRetainer) { + //TODO implement me + panic("implement me") +} + +func (r *localRetainer) Import(source InternRetainer) { + //TODO implement me + panic("implement me") +} + func newRetainer() *localRetainer { return &localRetainer{ retentions: make(map[Refcounted]int32), @@ -22,8 +37,9 @@ func newRetainer() *localRetainer { } func (r *localRetainer) Reference(obj Refcounted) { - r.retentions[obj] += 1 + r.retentions[obj]++ } + func (r *localRetainer) ReleaseAllWith(callback func(obj Refcounted, count int32)) { for k, v := range r.retentions { callback(k, v) @@ -40,9 +56,8 @@ func (r *localRetainer) ReleaseAll() { func TestInternLoadOrStoreValue(t *testing.T) { assert := assert.New(t) - sInterner, err := NewKeyedStringInterner(3, -1, true) + sInterner := NewKeyedStringInternerMemOnly(3) retainer := newRetainer() - assert.NoError(err) foo := []byte("foo") bar := []byte("bar") @@ -61,30 +76,31 @@ func TestInternLoadOrStoreValue(t *testing.T) { // Now test that the retainer is correct assert.Equal(1, len(retainer.retentions)) for _, v := range retainer.retentions { - assert.Equal(4, v) + assert.Equal(4, int(v)) } - sInterner.Release(retainer) + retainer.ReleaseAll() assert.Equal(0, len(retainer.retentions)) } func TestInternLoadOrStorePointer(t *testing.T) { assert := assert.New(t) - sInterner := newStringInterner(4, "/tmp") + sInterner := NewKeyedStringInternerMemOnly(4) + retainer := newRetainer() foo := []byte("foo") bar := []byte("bar") boo := []byte("boo") - v := sInterner.loadOrStore(foo) + v := sInterner.loadOrStore(foo, "", retainer) assert.Equal("foo", v) - v2 := sInterner.loadOrStore(foo) + v2 := sInterner.loadOrStore(foo, "", retainer) assert.Equal(&v, &v2, "must point to the same address") - v2 = sInterner.loadOrStore(bar) + v2 = sInterner.loadOrStore(bar, "", retainer) assert.NotEqual(&v, &v2, "must point to a different address") - v3 := sInterner.loadOrStore(bar) + v3 := sInterner.loadOrStore(bar, "", retainer) assert.Equal(&v2, &v3, "must point to the same address") - v4 := sInterner.loadOrStore(boo) + v4 := sInterner.loadOrStore(boo, "", retainer) assert.NotEqual(&v, &v4, "must point to a different address") assert.NotEqual(&v2, &v4, "must point to a different address") assert.NotEqual(&v3, &v4, "must point to a different address") @@ -92,38 +108,62 @@ func TestInternLoadOrStorePointer(t *testing.T) { func TestInternLoadOrStoreReset(t *testing.T) { assert := assert.New(t) - sInterner := newStringInterner(4, "/tmp") - - sInterner.loadOrStore([]byte("foo")) - assert.Equal(1, len(sInterner.cache.strings)) - sInterner.loadOrStore([]byte("bar")) - sInterner.loadOrStore([]byte("bar")) - assert.Equal(2, len(sInterner.cache.strings)) - sInterner.loadOrStore([]byte("boo")) - assert.Equal(3, len(sInterner.cache.strings)) - sInterner.loadOrStore([]byte("far")) - sInterner.loadOrStore([]byte("far")) - sInterner.loadOrStore([]byte("far")) + sInterner := NewKeyedStringInternerVals(1, true, "/", 512, 4) + retainer := newRetainer() + cacheLen := func() int { + internerUntyped, ok := sInterner.interners.Get("") + if !ok { + return 0 + } + interner := internerUntyped.(*stringInterner) + return len(interner.cache.strings) + } + assertCacheContains := func(s, comment string) { + internerUntyped, ok := sInterner.interners.Get("") + if !ok { + assert.Fail("No interner to hold key: " + comment) + } + interner := internerUntyped.(*stringInterner) + assert.Contains(interner.cache.strings, s, comment) + } + + assertCacheNotContains := func(s, comment string) { + internerUntyped, ok := sInterner.interners.Get("") + if !ok { + assert.Fail("No interner to hold key: " + comment) + } + interner := internerUntyped.(*stringInterner) + assert.NotContainsf(interner.cache.strings, s, comment) + } + + sInterner.loadOrStore([]byte("foo"), "", retainer) + assert.Equal(1, cacheLen()) + sInterner.loadOrStore([]byte("bar"), "", retainer) + sInterner.loadOrStore([]byte("bar"), "", retainer) + assert.Equal(2, cacheLen()) + sInterner.loadOrStore([]byte("boo"), "", retainer) + assert.Equal(3, cacheLen()) + sInterner.loadOrStore([]byte("far"), "", retainer) + sInterner.loadOrStore([]byte("far"), "", retainer) + sInterner.loadOrStore([]byte("far"), "", retainer) // Foo is the 4th-least recently used. - assert.Contains(sInterner.cache.strings, "foo", "first element still in cache") - assert.Equal(4, len(sInterner.cache.strings)) - sInterner.loadOrStore([]byte("val")) + assertCacheContains("foo", "first element still in cache") + assert.Equal(4, cacheLen()) + sInterner.loadOrStore([]byte("val"), "", retainer) // Something got bumped - assert.Equal(4, len(sInterner.cache.strings)) + assert.Equal(4, cacheLen()) // Foo was it. - assert.NotContains(sInterner.cache.strings, "foo", "oldest element evicted") - sInterner.loadOrStore([]byte("val")) - assert.Equal(4, len(sInterner.cache.strings)) + assertCacheNotContains("foo", "oldest element evicted") + sInterner.loadOrStore([]byte("val"), "", retainer) + assert.Equal(4, cacheLen()) } func NoTestLoadSeveralGenerations(t *testing.T) { - assert := assert.New(t) - interner, err := NewKeyedStringInterner(8, -1, true) - assert.NoError(err) + interner := NewKeyedStringInternerMemOnly(8) retainer := newRetainer() // Start generating random strings until we fill a few gigabytes of memory. - var totalUsed uint64 = 0 + var totalUsed uint64 for totalUsed < (64 * 1073741824) { text := make([]byte, 64) interner.LoadOrStore(text, "", retainer) @@ -131,5 +171,5 @@ func NoTestLoadSeveralGenerations(t *testing.T) { totalUsed += uint64(len(s)) } - interner.Release(retainer) + retainer.ReleaseAll() } diff --git a/pkg/util/cache/lru_cache.go b/pkg/util/cache/lru_cache.go index 6c2ce57955c43..1fd92ff69da80 100644 --- a/pkg/util/cache/lru_cache.go +++ b/pkg/util/cache/lru_cache.go @@ -1,9 +1,13 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + package cache -/*import ( - "github.com/DataDog/datadog-agent/pkg/config/utils" +import ( "github.com/DataDog/datadog-agent/pkg/telemetry" - "github.com/DataDog/datadog-agent/pkg/util/log" + "unsafe" ) var ( @@ -25,7 +29,7 @@ var ( tlmSIRStrBytes = telemetry.NewSimpleHistogram("dogstatsd", "string_interner_str_bytes", "Number of times string with specific length were added", []float64{1, 2, 4, 8, 16, 32, 64, 128}) -)*/ +) // stringInterner is a string cache providing a longer life for strings, // helping to avoid GC runs because they're re-used many times instead of @@ -49,7 +53,7 @@ type lruStringCache struct { func newLruStringCache(maxSize int, tlmEnabled bool) lruStringCache { if tlmEnabled { - //tlmSIRNew.Inc() + tlmSIRNew.Inc() } return lruStringCache{ strings: make(map[string]*stringCacheItem), @@ -62,7 +66,9 @@ func newLruStringCache(maxSize int, tlmEnabled bool) lruStringCache { // if not, it may evict the least recently used entry to make space. It uses the allocator arg function // to create the string, in case there's a separate backing store for it. func (c *lruStringCache) lookupOrInsert(key []byte, allocator func(key []byte) string) string { - // TODO: replace with a wrap over hashicorp/golang-lru. It's not less code, but less to test. + if len(key) < 1 { + return "" + } // here is the string interner trick: the map lookup using // string(key) doesn't actually allocate a string, but is @@ -71,7 +77,7 @@ func (c *lruStringCache) lookupOrInsert(key []byte, allocator func(key []byte) s // See https://github.com/golang/go/commit/f5f5a8b6209f84961687d993b93ea0d397f5d5bf if s, found := c.strings[string(key)]; found { if c.tlmEnabled { - //tlmSIRHits.Inc() + tlmSIRHits.Inc() } // If we found it, it's now the least recently used item, so rearrange it // in the LRU linked list. @@ -110,9 +116,9 @@ func (c *lruStringCache) lookupOrInsert(key []byte, allocator func(key []byte) s delete(c.strings, last.s) if c.tlmEnabled { - /* tlmSIResets.Inc() - tlmSIRBytes.Sub(float64(c.curBytes)) - tlmSIRSize.Sub(float64(len(c.strings))) */ + tlmSIResets.Inc() + tlmSIRBytes.Sub(float64(c.curBytes)) + tlmSIRSize.Sub(float64(len(c.strings))) c.curBytes -= lastLen } @@ -127,7 +133,7 @@ func (c *lruStringCache) lookupOrInsert(key []byte, allocator func(key []byte) s // Insert into the cache, and allocate to the backing store. s := &stringCacheItem{ - s: allocator(key), + s: str, prev: nil, next: c.head, } @@ -140,14 +146,17 @@ func (c *lruStringCache) lookupOrInsert(key []byte, allocator func(key []byte) s c.tail = s } - c.strings[s.s] = s + if str == "" || unsafe.StringData(str) == nil || c == nil || c.strings == nil { + panic("Dead string going to LRU") + } + c.strings[str] = s if c.tlmEnabled { length := len(s.s) - /*tlmSIRMiss.Inc() + tlmSIRMiss.Inc() tlmSIRSize.Inc() tlmSIRBytes.Add(float64(length)) - tlmSIRStrBytes.Observe(float64(length)) */ + tlmSIRStrBytes.Observe(float64(length)) c.curBytes += length } diff --git a/pkg/util/cache/lru_cache_test.go b/pkg/util/cache/lru_cache_test.go index 7d7675e68df6c..aef33353726a3 100644 --- a/pkg/util/cache/lru_cache_test.go +++ b/pkg/util/cache/lru_cache_test.go @@ -1,3 +1,8 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + package cache import ( diff --git a/pkg/util/cache/mmap_hash.go b/pkg/util/cache/mmap_hash.go index 771940c9d0e32..ac97d522001ea 100644 --- a/pkg/util/cache/mmap_hash.go +++ b/pkg/util/cache/mmap_hash.go @@ -1,412 +1,56 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + +//go:build !linux + package cache import ( "errors" - "fmt" - "golang.org/x/exp/slices" - "golang.org/x/text/language" - "golang.org/x/text/message" - "hash/maphash" - "io" - "io/fs" - "math" - "os" - "path/filepath" - "regexp" - "runtime" - "strings" - "sync" - "syscall" - "unsafe" - - log "github.com/DataDog/datadog-agent/pkg/util/log" ) -// hashPage is a 4k page of memory. After two bookkeeping ints, -// it has an array of index entries, followed by the strings themselves. -// The index entries grow from the start of the 4k page to the end, -// and the strings grow from the end of the 4k page to the start. The -// bookkeeping entries make sure they don't collide. -const hashPageSize = 4096 -const numEntries = (hashPageSize - 8) / 8 -const callStackDepth = 8 - -// MaxValueSize is the largest possible value we can store. Start with the page size and take off 16: -// 8 (4+2+2) for a single hashEntry, and 8 for the two int32s on the top of hashPage -const MaxValueSize = hashPageSize - 16 - -// MaxProbes is the maximum number of probes going into the hash table. -const MaxProbes = 8 - -// containerSummaryPattern will help abbreviate the name of the container. -var containerSummaryPattern = regexp.MustCompile("^container_id://[0-9a-f]{60}([0-9a-f]{4})") - -type hashEntry struct { - hashCode uint32 - offset, length uint16 -} - -type hashPage struct { - indexEntries, stringData int32 - // This array isn't actually this long. It's 'indexEntries' long - // and the tail is getting overwritten with strings. Adding a string grows - // in two directions simultaneously: the entry is added to the front - // of the page, and the string itself is prepended to the end. - // The offset field of each hashEntry object is relative to the - // address of the hashPage. - entries [numEntries]hashEntry -} - -func (hp *hashPage) insertAtIndex(index, hashcode int, key []byte) bool { - const entSize = int32(unsafe.Sizeof(hp.entries[0])) - remaining := hashPageSize - (hp.indexEntries*entSize + hp.stringData) - if remaining < (entSize + int32(len(key))) { - return false - } - copy(hp.entries[index+1:hp.indexEntries+1], hp.entries[index:hp.indexEntries]) - offset := hashPageSize - int(hp.stringData) - len(key) - stringBuf := unsafe.Slice((*byte)(unsafe.Pointer(hp)), hashPageSize) - copy(stringBuf[offset:offset+len(key)], key) - hp.entries[index].hashCode = uint32(hashcode) - hp.entries[index].length = uint16(len(key)) - hp.entries[index].offset = uint16(offset) - hp.indexEntries += 1 - hp.stringData += int32(len(key)) - return true -} - -// lookupOrInsert returns the allocated string and true, if it allocated. It -// returns empty string if it didn't fit here, and false. That means we treat -// this as a hash collision and find another page to look into (or insert) -func (hp *hashPage) lookupOrInsert(hcode uint64, key []byte) (string, bool) { - maskCode := func(hc uint64) int { - return int(hc & 0xFFFFFFFF) - } - maskedHCode := maskCode(hcode) - index, found := slices.BinarySearchFunc(hp.entries[:hp.indexEntries], hcode, - func(ent hashEntry, hc uint64) int { - return int(ent.hashCode) - maskCode(hc) - }) - if !found { - if !hp.insertAtIndex(index, maskedHCode, key) { - return "", false - } - } - return unsafe.String((*byte)(unsafe.Add(unsafe.Pointer(hp), hp.entries[index].offset)), - int(hp.entries[index].length)), !found -} - -type mmap_hash struct { - Name string - fd fs.File - used, capacity int64 // Bytes used and capacity for strings in the - seeds []maphash.Seed - seedHist []uint64 // Histograms of lookups that succeeded with the Nth seed. - pages []hashPage - mapping []byte // This is virtual address space, not memory used. - closeOnRelease bool - // value-length statistics, Welford's online variance algorithm - valueCount uint64 - valueMean float64 - valueM2 float64 - lock sync.Mutex -} - -// mmap_all_record holds every mmap_hash created. This isn't for permanent use, -// just debugging and validation. -type mmap_all_record struct { - // When we actually delete, make this nil. - hashes []*mmap_hash - origins map[string]int - lock sync.Mutex -} - -var all_mmaps = mmap_all_record{ - hashes: make([]*mmap_hash, 0, 1), - origins: make(map[string]int), -} - -func normalizeOrigin(origin string) string { - result := strings.Builder{} - for _, c := range origin { - switch c { - case '/': - fallthrough - case ':': - fallthrough - case ' ': - result.WriteRune('_') - default: - result.WriteRune(c) - } - } - return result.String() -} - -func newMmapHash(origin string, fileSize int64, prefixPath string, closeOnRelease bool) (*mmap_hash, error) { - if fileSize < hashPageSize { - return nil, errors.New("file size too small") - } - file, err := os.OpenFile(filepath.Join(prefixPath, fmt.Sprintf("%s-%d.dat", normalizeOrigin(origin), fileSize)), - os.O_RDWR|os.O_CREATE, 0755) - if err != nil { - return nil, err - } - - // Delete the file so that only our open FD keeps the inode alive. - defer func(name string) { - _ = os.Remove(name) - }(file.Name()) - - // Create the file, make a hole in it, mmap the hole. - if _, err = syscall.Seek(int(file.Fd()), int64(fileSize-1), io.SeekStart); err != nil { - return nil, err - } - // The hole requires a real byte after it to materialize. - if _, err = file.Write(make([]byte, 1)); err != nil { - return nil, err - } - - mappedAddresses, err := syscall.Mmap(int(file.Fd()), 0, int(fileSize), syscall.PROT_WRITE|syscall.PROT_READ, - syscall.MAP_SHARED|syscall.MAP_FILE) - if err != nil { - return nil, err - } - seeds := make([]maphash.Seed, 0, MaxProbes) - for i := 0; i < MaxProbes; i++ { - seeds = append(seeds, maphash.MakeSeed()) - } - - h := &mmap_hash{ - Name: origin, - fd: file, - used: 0, - capacity: fileSize, - mapping: mappedAddresses, - pages: unsafe.Slice((*hashPage)(unsafe.Pointer(&mappedAddresses[0])), fileSize/hashPageSize), - seeds: seeds, - seedHist: make([]uint64, len(seeds)), - closeOnRelease: closeOnRelease, - } - - all_mmaps.lock.Lock() - defer all_mmaps.lock.Unlock() - all_mmaps.hashes = append(all_mmaps.hashes, h) - - return h, nil -} - -// lookupOrInsert returns a pre-existing or newly created string with the value of key. It also -// returns a bool indicating whether implementation is full. If you get an empty string and a true, -// you would be able to allocate this string on some other instance that isn't full. If you get an -// empty string and a false, the implementation doesn't support this string. Go ahead and -// heap-allocate the string, then. -func (table *mmap_hash) lookupOrInsert(key []byte) (string, bool) { - keyLen := len(key) - if keyLen > MaxValueSize { - // We don't support long strings, punt. - return "", false - } - - if table.mapping == nil { - // We don't return strings after finalization. - _ = log.Errorf("Attempted to use mmap hash after release!") +// MaxValueSize is the largest possible value we can store. +const MaxValueSize = 4080 - // This will punt the error upwards, which will then allocate somewhere else. - return "", false - } - for n, seed := range table.seeds { - hash := maphash.Bytes(seed, key) - page := &table.pages[hash%uint64(len(table.pages))] - if result, allocated := page.lookupOrInsert(hash, key); result != "" { - if allocated { - // Online mean & variance calculation: - // https://en.wikipedia.org/wiki/Algorithms_for_calculating_variance#Welford's_online_algorithm - keyLenF := float64(keyLen) - table.used += int64(keyLen) - table.valueCount += 1 - delta := keyLenF - table.valueMean - table.valueMean += delta / float64(table.valueCount) - delta2 := keyLenF - table.valueMean - table.valueM2 += delta * delta2 - } - table.seedHist[n] += 1 - return result, false - } - } - return "", true -} - -func (table *mmap_hash) sizes() (int64, int64) { - return table.used, table.capacity -} +// Stub implementation for mmap_hash for non-Linux platforms. -// stats returns the mean and variance for the lengths of keys inserted into -// this mmap_hash. When these values aren't defined, you get NaN back. -func (table *mmap_hash) stats() (float64, float64) { - if table.valueCount < 1 { - return math.NaN(), math.NaN() - } else if table.valueCount < 2 { - return table.valueMean, math.NaN() - } else { - return table.valueMean, table.valueM2 / float64(table.valueCount) - } +// Report the mmap hashes in use and any failed checks. +func Report() { + // Nothing to report on. } -func (table *mmap_hash) name() string { - if table.Name[0] == '!' { - return table.Name - } else { - return containerSummaryPattern.ReplaceAllString(table.Name, "container-$1") - } +type mmapHash struct { } -func (table *mmap_hash) finalize() { - table.lock.Lock() - defer table.lock.Unlock() - if table.pages == nil { - _ = log.Warnf("finalize(%p): Already dead.", table) - } - address := unsafe.SliceData(table.mapping) - _ = log.Warnf(fmt.Sprintf("finalize(%p): Invalidating address %p-%p.", - table, address, unsafe.Add(unsafe.Pointer(address), len(table.mapping)))) - // Make the segment read-only, worry about actual deletion after we have - // better debugging around page faults. - var err error - if table.closeOnRelease { - err = syscall.Munmap(table.mapping) - if err != nil { - _ = log.Errorf("Failed munmap(): ", err) - } - err = table.fd.Close() - if err != nil { - _ = log.Errorf("Failed mapping file close(): ", err) - } - table.fd = nil - } else { - // Don't close the mapping, just mark it read-only. This leaks to disk and address space, but it's - // still better than using up heap. It also lets us track down reference leaks to this address - // space without crashing. - err = syscall.Mprotect(table.mapping, syscall.PROT_READ) - if err != nil { - _ = log.Errorf("Failed mprotect(): ", err) - } - } - table.pages = nil +// Name of a mmapHash. Based on origin. +func (*mmapHash) Name() string { + return "unimplemented" } -func (table *mmap_hash) finalized() bool { - return table.pages == nil +func (*mmapHash) lookupOrInsert(key []byte) (string, bool) { + return string(key), false } -func (table *mmap_hash) accessible() bool { - return table.fd != nil +func (*mmapHash) finalize() { } -// isMapped returns (index, active, safe) for the string s. If the address is mapped, -// index >= 0 (else -1). And if that mapping is still active, we get active=true. If -// the address is still mapped in the process address space (active or not), we get safe=true. -// Caller must hold lock to all_maps.lock. If index < 0, the other two return values are -// irrelevant. -func isMapped(s string) (int, bool, bool) { - // TODO: make isMapped lock-free. - addr := uintptr(unsafe.Pointer(unsafe.StringData(s))) - var constP *byte = nil - for n, t := range all_mmaps.hashes { - t.lock.Lock() - mapAddr := uintptr(unsafe.Pointer(unsafe.SliceData(t.mapping))) - if mapAddr <= addr && addr <= (mapAddr+unsafe.Sizeof(constP)*uintptr(len(t.mapping))) { - // Found it. - active, safe := !t.finalized(), t.accessible() - t.lock.Unlock() - return n, active, safe - } - t.lock.Unlock() - } - // address isn't part of our memory mapping, so it's safe to return. - return -1, false, true +func (*mmapHash) sizes() (int64, int64) { + return 0, 0 } -// logFailedCheck returns a safe value for 'tag'. Using the 'safe' value from isMapped, -// logFailedCheck will log a failed call to isMapped and -func logFailedCheck(index int, safe bool, callsite, tag string) string { - var name = all_mmaps.hashes[index].name() - - location := fmt.Sprintf("<%s>", name) - for i := 0; i < callStackDepth; i++ { - // skip over logFailedCheck and the in-package call site, just the ones above. - _, file, line, _ := runtime.Caller(2 + i) - location = fmt.Sprintf("%s\t[%s:%d]", location, - strings.Replace(file, "/go/src/github.com/DataDog/datadog-agent/pkg", "PKG", 1), line) - } - if _, found := all_mmaps.origins[location]; !found { - if safe { - _ = log.Errorf("mmap_hash.%v: Found tag (%s) from dead region, called from %v", callsite, tag, location) - } else { - _ = log.Errorf("mmap_hash.%v: Found tag (INACCESSIBLE) from dead region, called from %v", callsite, location) - } - } - all_mmaps.origins[location] += 1 - if safe { - return tag - } else { - return location - } +func newMmapHash(origin string, fileSize int64, prefixPath string, closeOnRelease bool) (*mmapHash, error) { + return nil, errors.New("unsupported platform for mmap hash") } +// Check a string to make sure it's still valid. Save a histogram of failures for tracking func Check(tag string) bool { - all_mmaps.lock.Lock() - defer all_mmaps.lock.Unlock() - - index, active, safe := isMapped(tag) - if index >= 0 && !active { - logFailedCheck(index, safe, "Check", tag) - } - return safe + return true } +// CheckDefault checks a string and returns it if it's valid, or returns an indicator of where +// it was called for debugging. func CheckDefault(tag string) string { - all_mmaps.lock.Lock() - defer all_mmaps.lock.Unlock() - index, active, safe := isMapped(tag) - if index >= 0 && !active { - return logFailedCheck(index, safe, "CheckDefault", tag) - } else { - return tag - } -} - -// Report the active and dead mappings, their lookup depths, and all the failed lookup checks. -func Report() { - all_mmaps.lock.Lock() - defer all_mmaps.lock.Unlock() - p := message.NewPrinter(language.English) - nrHashes := len(all_mmaps.hashes) - for n, t := range all_mmaps.hashes { - var status string - if t.finalized() { - status = "INACTIVE" - } else { - status = "ACTIVE" - } - mean, variance := t.stats() - log.Info(p.Sprintf("> %d/%d: %8s Origin=\"%s\" mmap range starting at %p: %v bytes.", n+1, nrHashes, status, - t.Name, unsafe.Pointer(unsafe.SliceData(t.mapping)), len(t.mapping))) - log.Info(p.Sprintf(" %d/%d: used: %11d, capacity: %11d. Utilization: %4.1f%%. Mean len: %4.2f, "+ - "Var len: %4.2f. Lookup depth: %10d %6d %5d %5d %3d %2d %2d %d", - n+1, nrHashes, t.used, t.capacity, 100.0*float64(t.used)/float64(t.capacity), mean, variance, - t.seedHist[0], t.seedHist[1], t.seedHist[2], t.seedHist[3], - t.seedHist[4], t.seedHist[5], t.seedHist[6], t.seedHist[7], - )) - } - - nrChecks := len(all_mmaps.origins) - count := 1 - totalFailedChecks := 0 - for k, v := range all_mmaps.origins { - log.Info(p.Sprintf("- %3d/%d %12d/ failed checks: %s", count, nrChecks, v, k)) - totalFailedChecks += v - count += 1 - } - log.Info(p.Sprintf("Failed Checks Total %d on %d different locations", totalFailedChecks, len(all_mmaps.origins))) + return tag } diff --git a/pkg/util/cache/mmap_hash_linux.go b/pkg/util/cache/mmap_hash_linux.go new file mode 100644 index 0000000000000..b330fc127863f --- /dev/null +++ b/pkg/util/cache/mmap_hash_linux.go @@ -0,0 +1,531 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + +package cache + +import ( + "errors" + "fmt" + "github.com/cihub/seelog" + "golang.org/x/exp/slices" + "golang.org/x/text/language" + "golang.org/x/text/message" + "hash/maphash" + "io" + "io/fs" + "math" + "os" + "path/filepath" + "regexp" + "runtime" + "strings" + "sync" + "syscall" + "unsafe" + + "github.com/DataDog/datadog-agent/pkg/util/log" +) + +// hashPage is a 4k page of memory. After two bookkeeping ints, +// it has an array of index entries, followed by the strings themselves. +// The index entries grow from the start of the 4k page to the end, +// and the strings grow from the end of the 4k page to the start. The +// bookkeeping entries make sure they don't collide. +const hashPageSize = 4096 +const numEntries = (hashPageSize - 8) / 8 +const callStackDepth = 8 + +// backBufferSize is padding on each page to handle any memory accesses off +// the end of the last page. From debugging SIGSEGVs: some string functions are +// implemented with SIMD instructions that read in multiples of 8 bytes. They +// read past the end of the string and then (presumably) mask off the bytes they +// don't care about. To avoid seg-faults, keep the last 8 bytes of the buffer +// unused to make sure any reads of short strings don't read off the end of +// allowable address space. Use 8 bytes in case some loops read the next whole word +// instead of stopping appropriately. +const backBufferSize = 8 + +// MaxValueSize is the largest possible value we can store. Start with the page size and take off 16: +// 8 (4+2+2) for a single hashEntry, 8 for the two int32s on the top of hashPage, and 8 for padding. +const MaxValueSize = hashPageSize - 16 - backBufferSize + +// MaxProbes is the maximum number of probes going into the hash table. +const MaxProbes = 8 + +const maxFailedPointers = 2000000 +const maxFailedPointersToPrint = 50 + +// packageSourceDir is the common prefix for source files found during callstack lookup +const packageSourceDir = "/go/src/github.com/DataDog/datadog-agent/pkg" + +// containerSummaryPattern will help abbreviate the name of the container. +var containerSummaryPattern = regexp.MustCompile("^container_id://[0-9a-f]{60}([0-9a-f]{4})") + +type hashEntry struct { + hashCode uint32 + offset, length uint16 +} + +type hashPage struct { + indexEntries, stringData int32 + // This array isn't actually this long. It's 'indexEntries' long + // and the tail is getting overwritten with strings. Adding a string grows + // in two directions simultaneously: the entry is added to the front + // of the page, and the string itself is prepended to the end. + // The offset field of each hashEntry object is relative to the + // address of the hashPage. + entries [numEntries]hashEntry +} + +func (hp *hashPage) insertAtIndex(index, hashcode int, key []byte) bool { + const entSize = int32(unsafe.Sizeof(hp.entries[0])) + if hp.stringData < backBufferSize { + hp.stringData = backBufferSize + } + remaining := hashPageSize - (hp.indexEntries*entSize + hp.stringData) + if remaining < (entSize + int32(len(key))) { + return false + } + copy(hp.entries[index+1:hp.indexEntries+1], hp.entries[index:hp.indexEntries]) + offset := hashPageSize - int(hp.stringData) - len(key) + stringBuf := unsafe.Slice((*byte)(unsafe.Pointer(hp)), hashPageSize) + copy(stringBuf[offset:offset+len(key)], key) + hp.entries[index].hashCode = uint32(hashcode) + hp.entries[index].length = uint16(len(key)) + hp.entries[index].offset = uint16(offset) + hp.indexEntries++ + hp.stringData += int32(len(key)) + return true +} + +// lookupOrInsert returns the allocated string and true, if it allocated. It +// returns empty string if it didn't fit here, and false. That means we treat +// this as a hash collision and find another page to look into (or insert) +func (hp *hashPage) lookupOrInsert(hcode uint64, key []byte) (string, bool) { + maskCode := func(hc uint64) int { + return int(hc & 0xFFFFFFFF) + } + maskedHCode := maskCode(hcode) + index, found := slices.BinarySearchFunc(hp.entries[:hp.indexEntries], hcode, + func(ent hashEntry, hc uint64) int { + return int(ent.hashCode) - maskCode(hc) + }) + if !found { + if !hp.insertAtIndex(index, maskedHCode, key) { + return "", false + } + } + return unsafe.String((*byte)(unsafe.Add(unsafe.Pointer(hp), hp.entries[index].offset)), + int(hp.entries[index].length)), !found +} + +type mmapHash struct { + name string + closed bool + fd fs.File + used, capacity int64 // Bytes used and capacity for strings in the + seeds []maphash.Seed + seedHist []uint64 // Histograms of lookups that succeeded with the Nth seed. + pages []hashPage + mapping []byte // This is virtual address space, not memory used. + closeOnRelease bool + // value-length statistics, Welford's online variance algorithm + valueCount uint64 + valueMean float64 + valueM2 float64 + lock sync.Mutex +} + +type failedPointer struct { + origin string + count int +} + +// mmapAllRecord holds every mmapHash created. This isn't for permanent use, +// just debugging and validation. +type mmapAllRecord struct { + // When we actually delete, make this nil. + hashes []*mmapHash + origins map[string]int + pointers map[uintptr]failedPointer + lock sync.Mutex +} + +var allMmaps = mmapAllRecord{ + hashes: make([]*mmapHash, 0, 1), + origins: make(map[string]int), + pointers: make(map[uintptr]failedPointer), +} + +func normalizeOrigin(origin string) string { + result := strings.Builder{} + for _, c := range origin { + switch c { + case '!': + fallthrough + case '/': + fallthrough + case ':': + fallthrough + case ' ': + result.WriteRune('_') + default: + result.WriteRune(c) + } + } + return result.String() +} + +func newMmapHash(origin string, fileSize int64, prefixPath string, closeOnRelease bool) (*mmapHash, error) { + if fileSize < hashPageSize { + return nil, errors.New("file size too small") + } + allMmaps.lock.Lock() + defer allMmaps.lock.Unlock() + + file, err := os.OpenFile(filepath.Join(prefixPath, fmt.Sprintf("%s-%d-%d.dat", normalizeOrigin(origin), len(allMmaps.hashes), fileSize)), + os.O_RDWR|os.O_CREATE, 0755) + if err != nil { + return nil, err + } + + // Delete the file so that only our open FD keeps the inode alive. + defer func(name string) { + _ = os.Remove(name) + }(file.Name()) + + // Create the file, make a hole in it, mmap the hole. + if _, err = syscall.Seek(int(file.Fd()), int64(fileSize-1), io.SeekStart); err != nil { + return nil, err + } + // The hole requires a real byte after it to materialize. + if _, err = file.Write(make([]byte, 1)); err != nil { + return nil, err + } + + mappedAddresses, err := syscall.Mmap(int(file.Fd()), 0, int(fileSize), syscall.PROT_WRITE|syscall.PROT_READ, + syscall.MAP_SHARED|syscall.MAP_FILE) + if err != nil { + return nil, err + } + seeds := make([]maphash.Seed, 0, MaxProbes) + for i := 0; i < MaxProbes; i++ { + seeds = append(seeds, maphash.MakeSeed()) + } + + h := &mmapHash{ + name: origin, + closed: false, + fd: file, + used: 0, + capacity: fileSize, + mapping: mappedAddresses, + pages: unsafe.Slice((*hashPage)(unsafe.Pointer(&mappedAddresses[0])), fileSize/hashPageSize), + seeds: seeds, + seedHist: make([]uint64, len(seeds)), + closeOnRelease: closeOnRelease, + } + + allMmaps.hashes = append(allMmaps.hashes, h) + + return h, nil +} + +// lookupOrInsert returns a pre-existing or newly created string with the value of key. It also +// returns a bool indicating whether implementation is full. If you get an empty string and a true, +// you would be able to allocate this string on some other instance that isn't full. If you get an +// empty string and a false, the implementation doesn't support this string. Go ahead and +// heap-allocate the string, then. +func (table *mmapHash) lookupOrInsert(key []byte) (string, bool) { + keyLen := len(key) + if keyLen > MaxValueSize { + // We don't support long strings, punt. + return "", false + } + + if table.closed { + // We don't return strings after finalization. + _ = log.Error("Attempted to use mmap hash after release!") + + // This will punt the error upwards, which will then allocate somewhere else. + return "", false + } + for n, seed := range table.seeds { + hash := maphash.Bytes(seed, key) + page := &table.pages[hash%uint64(len(table.pages))] + if result, allocated := page.lookupOrInsert(hash, key); result != "" { + if allocated { + // Online mean & variance calculation: + // https://en.wikipedia.org/wiki/Algorithms_for_calculating_variance#Welford's_online_algorithm + keyLenF := float64(keyLen) + table.used += int64(keyLen) + table.valueCount++ + delta := keyLenF - table.valueMean + table.valueMean += delta / float64(table.valueCount) + delta2 := keyLenF - table.valueMean + table.valueM2 += delta * delta2 + } + table.seedHist[n]++ + return result, false + } + } + return "", true +} + +func (table *mmapHash) sizes() (int64, int64) { + return table.used, table.capacity +} + +// stats returns the mean and variance for the lengths of keys inserted into +// this mmapHash. When these values aren't defined, you get NaN back. +func (table *mmapHash) stats() (float64, float64) { + if table.valueCount < 1 { + return math.NaN(), math.NaN() + } else if table.valueCount < 2 { + return table.valueMean, math.NaN() + } + + return table.valueMean, table.valueM2 / float64(table.valueCount) +} + +// Name of the mmapHash, printable and slightly sanitized. +func (table *mmapHash) Name() string { + if len(table.name) == 0 { + return "" + } else if table.name[0] == '!' { + return table.name + } + return containerSummaryPattern.ReplaceAllString(table.name, "container-$1") +} + +func (table *mmapHash) finalize() { + table.lock.Lock() + defer table.lock.Unlock() + if table.closed { + _ = log.Warnf("finalize(%p): Already dead.", table) + return + } + + address := unsafe.SliceData(table.mapping) + var closeOnRelease string + if table.closeOnRelease { + closeOnRelease = "YES" + } else { + closeOnRelease = "NO" + } + log.Debugf(fmt.Sprintf("finalize(%s): Invalidating address %p-%p. Close-on-release=%s", + table.Name(), address, unsafe.Add(unsafe.Pointer(address), len(table.mapping)), + closeOnRelease)) + // Make the segment read-only, worry about actual deletion after we have + // better debugging around page faults. + var err error + if table.closeOnRelease { + err = syscall.Munmap(table.mapping) + if err != nil { + _ = log.Errorf("Failed munmap(): %v", err) + } + err = table.fd.Close() + if err != nil { + _ = log.Errorf("Failed mapping file close(): %v", err) + } + table.fd = nil + } else { + // Don't close the mapping, just mark it read-only. This leaks to disk and address space, but it's + // still better than using up heap. It also lets us track down reference leaks to this address + // space without crashing. + err = syscall.Mprotect(table.mapping, syscall.PROT_READ) + if err != nil { + _ = log.Errorf("Failed mprotect(): %v", err) + } + table.fd = nil + } +} + +func (table *mmapHash) finalized() bool { + return table.closed +} + +func (table *mmapHash) accessible() bool { + return !table.finalized() || !table.closeOnRelease +} + +type mapCheck struct { + index int + safe, active bool +} + +// invalid indicates that the check found a string that should not +// be used anymore. +func (m mapCheck) invalid() bool { + return m.index >= 0 && !m.active +} + +// isMapped returns (index, active, safe) for the string s. If the address is mapped, +// index >= 0 (else -1). And if that mapping is still active, we get active=true. If +// the address is still mapped in the process address space (active or not), we get safe=true. +// Caller must hold lock to all_maps.lock. If index < 0, the other two return values are +// irrelevant. +func isMapped(s string) mapCheck { + // TODO: make isMapped lock-free. + addr := uintptr(unsafe.Pointer(unsafe.StringData(s))) + var constP *byte + for n, t := range allMmaps.hashes { + t.lock.Lock() + mapAddr := uintptr(unsafe.Pointer(unsafe.SliceData(t.mapping))) + if mapAddr <= addr && addr <= (mapAddr+unsafe.Sizeof(constP)*uintptr(len(t.mapping))) { + // Found it. + inactive := t.finalized() + safe := t.accessible() + if inactive { + if entry, ok := allMmaps.pointers[mapAddr]; !ok { + if len(allMmaps.pointers) < maxFailedPointers { + allMmaps.pointers[mapAddr] = failedPointer{ + origin: t.Name(), + count: 1, + } + } + } else { + entry.count++ + allMmaps.pointers[mapAddr] = entry + } + } + t.lock.Unlock() + return mapCheck{ + index: n, + safe: safe, + active: !inactive, + } + } + t.lock.Unlock() + } + // address isn't part of our memory mapping, so it's safe to return. + return mapCheck{ + index: -1, + safe: true, + active: false, + } +} + +// logFailedCheck returns a safe value for 'tag'. Using the 'safe' value from isMapped, +// logFailedCheck will log a failed call to isMapped and +func logFailedCheck(check mapCheck, callsite, tag string) string { + location := fmt.Sprintf("<%s>", allMmaps.hashes[check.index].Name()) + for i := 0; i < callStackDepth; i++ { + // skip over logFailedCheck and the in-package call site, just the ones above. + _, file, line, _ := runtime.Caller(2 + i) + location = fmt.Sprintf("%s\t[%s:%d]", location, + strings.Replace(file, packageSourceDir, "PKG", 1), line) + } + if _, found := allMmaps.origins[location]; !found { + addr := unsafe.StringData(tag) + if check.safe { + log.Debugf("mmap_hash.%v: %p: Found tag (%s) from dead region, called from %v", callsite, addr, tag, location) + } else { + log.Debugf("mmap_hash.%v: %p: Found tag (INACCESSIBLE) from dead region, called from %v", callsite, addr, location) + } + } + allMmaps.origins[location]++ + if check.safe { + return tag + } + return allMmaps.hashes[check.index].Name() +} + +// Check a string to make sure it's still valid. Save a histogram of failures for tracking +func Check(tag string) bool { + allMmaps.lock.Lock() + defer allMmaps.lock.Unlock() + + check := isMapped(tag) + if check.invalid() { + logFailedCheck(check, "Check", tag) + return false + } + return check.safe +} + +// CheckDefault checks a string and returns it if it's valid, or returns an indicator of where +// it was called for debugging. +func CheckDefault(tag string) string { + allMmaps.lock.Lock() + defer allMmaps.lock.Unlock() + check := isMapped(tag) + if check.invalid() { + return logFailedCheck(check, "CheckDefault", tag) + } + return tag +} + +// Report the active and dead mappings, their lookup depths, and all the failed lookup checks. +func Report() { + level, err := log.GetLogLevel() + if err != nil { + // Weird, log the logging level. + _ = log.Errorf("Report: GetLogLevel: %v", err) + } else if level > seelog.DebugLvl { + // Nothing here will get printed, so don't bother doing the work. + return + } + allMmaps.lock.Lock() + defer allMmaps.lock.Unlock() + p := message.NewPrinter(language.English) + nrHashes := len(allMmaps.hashes) + type originData struct { + name string + totalValues uint64 + totalActiveAllocated, totalAllocated int64 + } + + mapData := make(map[string]originData) + for n, t := range allMmaps.hashes { + var status string + name := t.Name() + data := mapData[name] + data.name = name + data.totalValues += t.valueCount + data.totalAllocated += t.capacity + if t.finalized() { + status = "INACTIVE" + } else { + data.totalActiveAllocated += t.capacity + status = "ACTIVE" + } + mapData[name] = data + + mean, variance := t.stats() + log.Debug(p.Sprintf("> %d/%d: %8s Origin=\"%s\" mmap range starting at %p: %v bytes."+ + " Used: %11d, capacity: %11d. Utilization: %4.1f%%. Mean len: %4.2f, "+ + "Stddev len: %4.2f. Lookup depth: %10d %6d %5d %5d %3d %2d %2d %d", n+1, nrHashes, status, + t.Name(), unsafe.Pointer(unsafe.SliceData(t.mapping)), len(t.mapping), + t.used, t.capacity, 100.0*float64(t.used)/float64(t.capacity), mean, math.Sqrt(variance), + t.seedHist[0], t.seedHist[1], t.seedHist[2], t.seedHist[3], + t.seedHist[4], t.seedHist[5], t.seedHist[6], t.seedHist[7])) + } + + for k, v := range mapData { + log.Debug(p.Sprintf("* %40s: Total Values: %d, Active Bytes Allocated: %d, Total Bytes Allocated: %d", + k, v.totalValues, v.totalActiveAllocated, v.totalAllocated)) + } + + nrChecks := len(allMmaps.origins) + count := 1 + totalFailedChecks := 0 + for k, v := range allMmaps.origins { + log.Debug(p.Sprintf("- %3d/%d %12d/ failed checks: %s", count, nrChecks, v, k)) + totalFailedChecks += v + count++ + } + + if totalFailedChecks > 0 { + log.Info(p.Sprintf("Failed Checks Total %d on %d different locations", totalFailedChecks, len(allMmaps.origins))) + } + + if len(allMmaps.pointers) < maxFailedPointersToPrint { + for ptr, entry := range allMmaps.pointers { + log.Debug(p.Sprintf("Address 0x%016x in %s: %d hits", ptr, entry.origin, entry.count)) + } + } + log.Debugf(p.Sprintf("Too many (%d) pointers saved.", len(allMmaps.pointers))) +} diff --git a/pkg/util/cache/mmap_hash_test.go b/pkg/util/cache/mmap_hash_linux_test.go similarity index 74% rename from pkg/util/cache/mmap_hash_test.go rename to pkg/util/cache/mmap_hash_linux_test.go index 34ef8a8d02334..14a316bee5c4f 100644 --- a/pkg/util/cache/mmap_hash_test.go +++ b/pkg/util/cache/mmap_hash_linux_test.go @@ -1,3 +1,8 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + package cache import ( @@ -8,13 +13,13 @@ import ( ) func Test_mmap_hash(t *testing.T) { - table, err := newMmapHash(8192, "/tmp") + table, err := newMmapHash("", 8192, "/tmp", false) assert.NoError(t, err) foo, _ := table.lookupOrInsert([]byte("foo")) bar, _ := table.lookupOrInsert([]byte("bar")) tooLong := make([]byte, 4200) // larger than 4096 - for i, _ := range tooLong { + for i := range tooLong { tooLong[i] = byte(i % 256) } diff --git a/pkg/util/cache/refcount.go b/pkg/util/cache/refcount.go index 200d0284e1f6e..5404cbb7590bc 100644 --- a/pkg/util/cache/refcount.go +++ b/pkg/util/cache/refcount.go @@ -1,26 +1,38 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + package cache import ( "golang.org/x/text/language" "golang.org/x/text/message" "strings" + "sync" ) // Refcounted tracks references. The interface doesn't provide a reference construction // mechanism, use the per-type implementation functions for that. type Refcounted interface { + // Retain n additional references + Retain(n int32) + // Release n references. Release(n int32) + // Name of the object. Name() string } // InternRetainer counts references to internal types, that we can then release later. Here, it's // used for stringInterner instances. type InternRetainer interface { + // Reference the obj once Reference(obj Refcounted) + // ReferenceN References obj n times. ReferenceN(obj Refcounted, n int32) // CopyTo duplicates all references in this InternRetainer into dest. CopyTo(dest InternRetainer) - // Import takes all the references from source. Source will hae no references after this operation. + // Import takes all the references from source. Source will have no references after this operation. Import(source InternRetainer) // ReleaseAll calls Release once on every object it's ever Referenced for each time it was referenced. ReleaseAll() @@ -35,9 +47,17 @@ type SmallRetainer struct { counts []int32 } +// Len has the number of origins. +func (s *SmallRetainer) Len() int { + return len(s.origins) +} + +// Reference an object once. func (s *SmallRetainer) Reference(obj Refcounted) { s.ReferenceN(obj, 1) } + +// ReferenceN an object n times func (s *SmallRetainer) ReferenceN(obj Refcounted, n int32) { if s.origins == nil { s.origins = make([]Refcounted, 0, 2) @@ -55,6 +75,8 @@ func (s *SmallRetainer) ReferenceN(obj Refcounted, n int32) { s.counts = append(s.counts, n) } +// ReleaseAllWith uses callback to release everything it holds, then forgets what +// it has. func (s *SmallRetainer) ReleaseAllWith(callback func(obj Refcounted, count int32)) { if s.origins == nil { return @@ -66,75 +88,117 @@ func (s *SmallRetainer) ReleaseAllWith(callback func(obj Refcounted, count int32 s.counts = s.counts[:0] } +// ReleaseAll releases every object. func (s *SmallRetainer) ReleaseAll() { s.ReleaseAllWith(func(obj Refcounted, count int32) { obj.Release(count) }) } +// CopyTo copies every reference this RetainerBlock has to other, creating +// additional references to each Refcounted in the process. func (s *SmallRetainer) CopyTo(dest InternRetainer) { for n, o := range s.origins { + o.Retain(s.counts[n]) dest.ReferenceN(o, s.counts[n]) } } +// Import another retainer's retentions. The other retainer loses +// its retentions after this. func (s *SmallRetainer) Import(other InternRetainer) { - other.ReleaseAllWith(s.ReferenceN) + if other != nil { + other.ReleaseAllWith(s.ReferenceN) + } +} + +// Drop all retentions and effectively leak. Useful for testing and nothing else. +func (s *SmallRetainer) Drop() { + s.counts = nil + s.origins = nil } +// RetainerBlock is a synchronized Retainer. type RetainerBlock struct { - InternRetainer retentions map[Refcounted]int32 + lock sync.Mutex } +// NewRetainerBlock allocates a RetainerBlock. func NewRetainerBlock() *RetainerBlock { return &RetainerBlock{ retentions: make(map[Refcounted]int32), + lock: sync.Mutex{}, } } +// Reference an object once. func (r *RetainerBlock) Reference(obj Refcounted) { - r.retentions[obj] += 1 + r.lock.Lock() + r.retentions[obj]++ + r.lock.Unlock() } +// ReferenceN an object n times. func (r *RetainerBlock) ReferenceN(obj Refcounted, n int32) { + r.lock.Lock() r.retentions[obj] += n + r.lock.Unlock() } +// ReleaseAllWith uses callback to release everything it holds, then forgets what +// it has. func (r *RetainerBlock) ReleaseAllWith(callback func(obj Refcounted, count int32)) { + r.lock.Lock() for k, v := range r.retentions { callback(k, v) delete(r.retentions, k) } + r.lock.Unlock() } +// ReleaseAll releases every object. func (r *RetainerBlock) ReleaseAll() { + // Don't lock as we're calling a locking method. r.ReleaseAllWith(func(obj Refcounted, count int32) { obj.Release(count) }) } +// Import another retainer's retentions. The other retainer loses +// its retentions after this. func (r *RetainerBlock) Import(other InternRetainer) { + r.lock.Lock() other.ReleaseAllWith(func(obj Refcounted, count int32) { r.retentions[obj] += count }) + r.lock.Unlock() } +// CopyTo copies every reference this RetainerBlock has to other, creating +// additional references to each Refcounted in the process. func (r *RetainerBlock) CopyTo(other InternRetainer) { + r.lock.Lock() for k, v := range r.retentions { + k.Retain(v) other.ReferenceN(k, v) } + r.lock.Unlock() } +// Summarize generates a string summary of what's summarized (via Name) and how many references +// each Refcounted gets. func (r *RetainerBlock) Summarize() string { + r.lock.Lock() p := message.NewPrinter(language.English) s := strings.Builder{} - var total int32 = 0 + var total int32 s.WriteString(p.Sprintf("{%d keys. ", len(r.retentions))) for k, v := range r.retentions { s.WriteString(p.Sprintf("%s: %d, ", k.Name(), v)) total += v } s.WriteString(p.Sprintf("; %d total}", total)) + r.lock.Unlock() return s.String() } diff --git a/rtloader/rtloader/api.cpp b/rtloader/rtloader/api.cpp index ec6fd2ed2f932..d6e3118e34b67 100644 --- a/rtloader/rtloader/api.cpp +++ b/rtloader/rtloader/api.cpp @@ -12,11 +12,6 @@ #ifndef _WIN32 // clang-format off // handler stuff -#if defined(__GLIBC__) -#define HAS_BACKTRACE_LIB (1) -#endif -#include - #ifdef HAS_BACKTRACE_LIB #include #endif @@ -380,46 +375,16 @@ static inline void core(int sig) all its C-context, before it unwinds as would be the case if we allowed the go runtime to handle it. */ -struct sigaction _prior_sa; # define STACKTRACE_SIZE 500 -void signalHandler(int sig, siginfo_t *siginfo, void *uctx) -{ - /* - * - /* Context to describe whole processor state. - typedef struct - { - gregset_t gregs; - /* Note that fpregs is a pointer. - fpregset_t fpregs; - unsigned long __reserved1 [8]; - } mcontext_t; - - /* Userlevel context. - typedef struct ucontext - { - unsigned long int uc_flags; - struct ucontext *uc_link; - stack_t uc_stack; - mcontext_t uc_mcontext; - __sigset_t uc_sigmask; - struct _libc_fpstate __fpregs_mem; - } ucontext_t; - - */ +void signalHandler(int sig, siginfo_t *, void *) +{ # ifdef HAS_BACKTRACE_LIB void *buffer[STACKTRACE_SIZE]; char **symbols; - ucontext_t *ucontext = (ucontext_t*) uctx; + size_t nptrs = backtrace(buffer, STACKTRACE_SIZE); # endif - #ifdef __aarch64__ - std::cerr << "HANDLER CAUGHT signal Error: signal " << sig << " from instruction at address " - << (void*) ucontext->uc_mcontext.pc << ", Signal Address: " << siginfo->si_addr << std::endl; - #else - std::cerr << "HANDLER CAUGHT signal Error: signal " << sig << " from instruction at address " - << (void*) ucontext->uc_mcontext.gregs[REG_RIP] << ", Signal Address: " << siginfo->si_addr << std::endl; - #endif + std::cerr << "HANDLER CAUGHT signal Error: signal " << sig << std::endl; # ifdef HAS_BACKTRACE_LIB symbols = backtrace_symbols(buffer, nptrs); if (symbols == NULL) { @@ -432,33 +397,15 @@ void signalHandler(int sig, siginfo_t *siginfo, void *uctx) _free(symbols); } -# else - std::cerr << "HAS_BACKTRACE_LIB is false, no backtrace support enabled." << std::endl; # endif - if (_prior_sa.sa_sigaction != NULL) { - _prior_sa.sa_sigaction(sig, siginfo, uctx); - } else if (_prior_sa.sa_handler != NULL) { - _prior_sa.sa_handler(sig); - } + // dump core if so configured __sync_synchronize(); if (core_dump) { core_dump(sig); } else { - // kill(getpid(), SIGABRT); + kill(getpid(), SIGABRT); } - - struct sigaction sa; - sa.sa_flags = 0; - sa.sa_handler = SIG_DFL; - int err = sigaction(SIGSEGV, &sa, &_prior_sa); - if (err) { - std::cerr << "unable to reset crash handler: " << strerror(errno) << std::endl; - } - kill(getpid(), SIGSEGV); - - // on segfault - what else? - } /* @@ -472,7 +419,7 @@ DATADOG_AGENT_RTLOADER_API int handle_crashes(const int enable, char **error) sa.sa_sigaction = signalHandler; // on segfault - what else? - int err = 0; /*sigaction(SIGSEGV, &sa, &_prior_sa); + int err = sigaction(SIGSEGV, &sa, NULL); if (enable && err == 0) { __sync_synchronize(); @@ -483,7 +430,7 @@ DATADOG_AGENT_RTLOADER_API int handle_crashes(const int enable, char **error) err_msg << "unable to set crash handler: " << strerror(errno); *error = strdupe(err_msg.str().c_str()); } -*/ + return err == 0 ? 1 : 0; } #endif diff --git a/test/benchmarks/aggregator/main.go b/test/benchmarks/aggregator/main.go index 6ed0a2fa5f974..d699f58382103 100644 --- a/test/benchmarks/aggregator/main.go +++ b/test/benchmarks/aggregator/main.go @@ -19,7 +19,6 @@ import ( log "github.com/cihub/seelog" - "github.com/DataDog/datadog-agent/comp/forwarder/defaultforwarder" "github.com/DataDog/datadog-agent/comp/forwarder/defaultforwarder/transaction" "github.com/DataDog/datadog-agent/pkg/aggregator" checkid "github.com/DataDog/datadog-agent/pkg/collector/check/id" @@ -205,7 +204,7 @@ func main() { f := &forwarderBenchStub{} s := serializer.NewSerializer(f, nil) - agg = aggregator.NewBufferedAggregator(s, nil, "hostname", time.Duration(*flushIval)*time.Second) + agg = aggregator.NewBufferedAggregator(s, nil, "hostname", time.Duration(*flushIval)*time.Second, nil) aggregator.SetDefaultAggregator(agg) sender, err := aggregator.GetSender(checkid.ID("benchmark check")) diff --git a/test/benchmarks/dogstatsd/main.go b/test/benchmarks/dogstatsd/main.go index f70301b0a28b9..a7ef7fd6369b2 100644 --- a/test/benchmarks/dogstatsd/main.go +++ b/test/benchmarks/dogstatsd/main.go @@ -217,7 +217,7 @@ func main() { mockConfig.Set("dogstatsd_stats_enable", true) mockConfig.Set("dogstatsd_stats_buffer", 100) s := serializer.NewSerializer(f, nil) - aggr := aggregator.NewBufferedAggregator(s, nil, "localhost", aggregator.DefaultFlushInterval) + aggr := aggregator.NewBufferedAggregator(s, nil, "localhost", aggregator.DefaultFlushInterval, nil) statsd, err := dogstatsd.NewServer(aggr.GetBufferedChannels(), false) if err != nil { log.Errorf("Problem allocating dogstatsd server: %s", err)