-
Notifications
You must be signed in to change notification settings - Fork 242
feat: Experimental Trace Metric #1116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
2dc99b2
c341e95
1f981fb
6ff23fb
67d4e1c
132f880
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| package sentry | ||
|
|
||
| import ( | ||
| "context" | ||
| "sync" | ||
| "time" | ||
| ) | ||
|
|
||
| type BatchMeter struct { | ||
| client *Client | ||
| metricsCh chan Metric | ||
| flushCh chan chan struct{} | ||
| cancel context.CancelFunc | ||
| wg sync.WaitGroup | ||
| startOnce sync.Once | ||
| shutdownOnce sync.Once | ||
| } | ||
|
|
||
| func NewBatchMeter(client *Client) *BatchMeter { | ||
| return &BatchMeter{ | ||
| client: client, | ||
| metricsCh: make(chan Metric, batchSize), | ||
| flushCh: make(chan chan struct{}), | ||
| } | ||
| } | ||
|
|
||
| func (l *BatchMeter) Start() { | ||
| l.startOnce.Do(func() { | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| l.cancel = cancel | ||
| l.wg.Add(1) | ||
| go l.run(ctx) | ||
| }) | ||
| } | ||
|
|
||
| func (l *BatchMeter) Flush(timeout <-chan struct{}) { | ||
| done := make(chan struct{}) | ||
| select { | ||
| case l.flushCh <- done: | ||
| select { | ||
| case <-done: | ||
| case <-timeout: | ||
| } | ||
| case <-timeout: | ||
| } | ||
| } | ||
|
|
||
| func (l *BatchMeter) Shutdown() { | ||
| l.shutdownOnce.Do(func() { | ||
| if l.cancel != nil { | ||
| l.cancel() | ||
| l.wg.Wait() | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| func (l *BatchMeter) run(ctx context.Context) { | ||
| defer l.wg.Done() | ||
| var metrics []Metric | ||
| timer := time.NewTimer(batchTimeout) | ||
| defer timer.Stop() | ||
|
|
||
| for { | ||
| select { | ||
| case metric := <-l.metricsCh: | ||
| metrics = append(metrics, metric) | ||
| if len(metrics) >= batchSize { | ||
| l.processEvent(metrics) | ||
| metrics = nil | ||
| if !timer.Stop() { | ||
| <-timer.C | ||
| } | ||
| timer.Reset(batchTimeout) | ||
| } | ||
| case <-timer.C: | ||
| if len(metrics) > 0 { | ||
| l.processEvent(metrics) | ||
| metrics = nil | ||
| } | ||
| timer.Reset(batchTimeout) | ||
| case done := <-l.flushCh: | ||
| flushDrain: | ||
| for { | ||
| select { | ||
| case metric := <-l.metricsCh: | ||
| metrics = append(metrics, metric) | ||
| default: | ||
| break flushDrain | ||
| } | ||
| } | ||
|
|
||
| if len(metrics) > 0 { | ||
| l.processEvent(metrics) | ||
| metrics = nil | ||
| } | ||
| if !timer.Stop() { | ||
| <-timer.C | ||
| } | ||
| timer.Reset(batchTimeout) | ||
| close(done) | ||
| case <-ctx.Done(): | ||
| drain: | ||
| for { | ||
| select { | ||
| case metric := <-l.metricsCh: | ||
| metrics = append(metrics, metric) | ||
| default: | ||
| break drain | ||
| } | ||
| } | ||
|
|
||
| if len(metrics) > 0 { | ||
| l.processEvent(metrics) | ||
| } | ||
| return | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func (l *BatchMeter) processEvent(metrics []Metric) { | ||
| event := NewEvent() | ||
| event.Timestamp = time.Now() | ||
| event.EventID = EventID(uuid()) | ||
| event.Type = logEvent.Type | ||
| event.Metrics = metrics | ||
| l.client.Transport.SendEvent(event) | ||
aldy505 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -234,6 +234,9 @@ type ClientOptions struct { | |
| Tags map[string]string | ||
| // EnableLogs controls when logs should be emitted. | ||
| EnableLogs bool | ||
| // ExperimentalEnableTraceMetric controls when trace metrics should be emitted. | ||
| // This is an experimental feature that is subject to change. | ||
| ExperimentalEnableTraceMetric bool | ||
| // TraceIgnoreStatusCodes is a list of HTTP status codes that should not be traced. | ||
| // Each element can be either: | ||
| // - A single-element slice [code] for a specific status code | ||
|
|
@@ -265,6 +268,7 @@ type Client struct { | |
| // not supported, create a new client instead. | ||
| Transport Transport | ||
| batchLogger *BatchLogger | ||
| batchMeter *BatchMeter | ||
| } | ||
|
|
||
| // NewClient creates and returns an instance of Client configured using | ||
|
|
@@ -369,6 +373,11 @@ func NewClient(options ClientOptions) (*Client, error) { | |
| client.batchLogger.Start() | ||
| } | ||
|
|
||
| if options.ExperimentalEnableTraceMetric { | ||
| client.batchMeter = NewBatchMeter(&client) | ||
| client.batchMeter.Start() | ||
| } | ||
|
|
||
| client.setupTransport() | ||
| client.setupIntegrations() | ||
|
|
||
|
|
@@ -531,7 +540,7 @@ func (client *Client) RecoverWithContext( | |
| // the network synchronously, configure it to use the HTTPSyncTransport in the | ||
| // call to Init. | ||
| func (client *Client) Flush(timeout time.Duration) bool { | ||
| if client.batchLogger != nil { | ||
| if client.batchLogger != nil || client.batchMeter != nil { | ||
| ctx, cancel := context.WithTimeout(context.Background(), timeout) | ||
| defer cancel() | ||
| return client.FlushWithContext(ctx) | ||
|
|
@@ -555,6 +564,9 @@ func (client *Client) FlushWithContext(ctx context.Context) bool { | |
| if client.batchLogger != nil { | ||
| client.batchLogger.Flush(ctx.Done()) | ||
| } | ||
| if client.batchMeter != nil { | ||
| client.batchMeter.Flush(ctx.Done()) | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Goroutine Leak in Client CloseThe Client.Close() method doesn't call Shutdown() on batchMeter (or batchLogger), causing goroutine leaks. The BatchMeter.run() goroutine will continue running indefinitely even after the client is closed, as it only stops when the context is cancelled. The Shutdown() method should be called to properly cancel the context and wait for the goroutine to finish, similar to how Flush() calls both batchLogger.Flush() and batchMeter.Flush(). |
||
| return client.Transport.FlushWithContext(ctx) | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Trace Metrics Misclassified as Logs
The
processEventmethod setsevent.TypetologEvent.Type. Since this method processes trace metrics, the event type should betraceMetricEvent.Type. This miscategorization causes trace metrics to be processed as log events, which can lead to incorrect backend processing or issues with the metrics pipeline.