Skip to content

Commit

Permalink
[chore] Reduce complexity in processorhelper obsreport (#10693)
Browse files Browse the repository at this point in the history
This PR removes the `recordData` method, which I believe is an unhelpful
abstraction for reporting internal processor metrics.

The drawbacks of the function are minor, but briefly:
1. Recording any metric requires passing in zeros for _all other
metrics_. This is cumbersome to extend because any new metric requires
updating the call to `recordData` for all metrics. It's also fragile
because if we have more than a few metrics it is easy to position the
intended metric incorrectly.
2. Every metric describes one data type, which is passed into
`recordData` but then just used as a switch to get back to code that is
specific to that data type.
3. Every call to `recordData` instantiates N variables, where N is the
number of metrics for each data type.

All of this seems unnecessary as we can just report each metric in one
unambiguous line of code.
  • Loading branch information
djaglowski authored Jul 22, 2024
1 parent 9ef6356 commit 49339e8
Showing 1 changed file with 16 additions and 42 deletions.
58 changes: 16 additions & 42 deletions processor/processorhelper/obsreport.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ func BuildCustomMetricName(configType, metric string) string {

// ObsReport is a helper to add observability to a processor.
type ObsReport struct {
otelAttrs []attribute.KeyValue
telemetryBuilder *metadata.TelemetryBuilder
otelAttrs []attribute.KeyValue
telBuilder *metadata.TelemetryBuilder
}

// ObsReportSettings are settings for creating an ObsReport.
Expand All @@ -48,100 +48,74 @@ func NewObsReport(cfg ObsReportSettings) (*ObsReport, error) {
}

func newObsReport(cfg ObsReportSettings) (*ObsReport, error) {
telemetryBuilder, err := metadata.NewTelemetryBuilder(cfg.ProcessorCreateSettings.TelemetrySettings, metadata.WithLevel(cfg.ProcessorCreateSettings.MetricsLevel))
telBuilder, err := metadata.NewTelemetryBuilder(cfg.ProcessorCreateSettings.TelemetrySettings, metadata.WithLevel(cfg.ProcessorCreateSettings.MetricsLevel))
if err != nil {
return nil, err
}
return &ObsReport{
otelAttrs: []attribute.KeyValue{
attribute.String(obsmetrics.ProcessorKey, cfg.ProcessorID.String()),
},
telemetryBuilder: telemetryBuilder,
telBuilder: telBuilder,
}, nil
}

func (or *ObsReport) recordData(ctx context.Context, dataType component.DataType, accepted, refused, dropped, inserted int64) {
var acceptedCount, refusedCount, droppedCount, insertedCount metric.Int64Counter
switch dataType {
case component.DataTypeTraces:
acceptedCount = or.telemetryBuilder.ProcessorAcceptedSpans
refusedCount = or.telemetryBuilder.ProcessorRefusedSpans
droppedCount = or.telemetryBuilder.ProcessorDroppedSpans
insertedCount = or.telemetryBuilder.ProcessorInsertedSpans
case component.DataTypeMetrics:
acceptedCount = or.telemetryBuilder.ProcessorAcceptedMetricPoints
refusedCount = or.telemetryBuilder.ProcessorRefusedMetricPoints
droppedCount = or.telemetryBuilder.ProcessorDroppedMetricPoints
insertedCount = or.telemetryBuilder.ProcessorInsertedMetricPoints
case component.DataTypeLogs:
acceptedCount = or.telemetryBuilder.ProcessorAcceptedLogRecords
refusedCount = or.telemetryBuilder.ProcessorRefusedLogRecords
droppedCount = or.telemetryBuilder.ProcessorDroppedLogRecords
insertedCount = or.telemetryBuilder.ProcessorInsertedLogRecords
}

acceptedCount.Add(ctx, accepted, metric.WithAttributes(or.otelAttrs...))
refusedCount.Add(ctx, refused, metric.WithAttributes(or.otelAttrs...))
droppedCount.Add(ctx, dropped, metric.WithAttributes(or.otelAttrs...))
insertedCount.Add(ctx, inserted, metric.WithAttributes(or.otelAttrs...))
}

// TracesAccepted reports that the trace data was accepted.
func (or *ObsReport) TracesAccepted(ctx context.Context, numSpans int) {
or.recordData(ctx, component.DataTypeTraces, int64(numSpans), int64(0), int64(0), int64(0))
or.telBuilder.ProcessorAcceptedSpans.Add(ctx, int64(numSpans), metric.WithAttributes(or.otelAttrs...))
}

// TracesRefused reports that the trace data was refused.
func (or *ObsReport) TracesRefused(ctx context.Context, numSpans int) {
or.recordData(ctx, component.DataTypeTraces, int64(0), int64(numSpans), int64(0), int64(0))
or.telBuilder.ProcessorRefusedSpans.Add(ctx, int64(numSpans), metric.WithAttributes(or.otelAttrs...))
}

// TracesDropped reports that the trace data was dropped.
func (or *ObsReport) TracesDropped(ctx context.Context, numSpans int) {
or.recordData(ctx, component.DataTypeTraces, int64(0), int64(0), int64(numSpans), int64(0))
or.telBuilder.ProcessorDroppedSpans.Add(ctx, int64(numSpans), metric.WithAttributes(or.otelAttrs...))
}

// TracesInserted reports that the trace data was inserted.
func (or *ObsReport) TracesInserted(ctx context.Context, numSpans int) {
or.recordData(ctx, component.DataTypeTraces, int64(0), int64(0), int64(0), int64(numSpans))
or.telBuilder.ProcessorInsertedSpans.Add(ctx, int64(numSpans), metric.WithAttributes(or.otelAttrs...))
}

// MetricsAccepted reports that the metrics were accepted.
func (or *ObsReport) MetricsAccepted(ctx context.Context, numPoints int) {
or.recordData(ctx, component.DataTypeMetrics, int64(numPoints), int64(0), int64(0), int64(0))
or.telBuilder.ProcessorAcceptedMetricPoints.Add(ctx, int64(numPoints), metric.WithAttributes(or.otelAttrs...))
}

// MetricsRefused reports that the metrics were refused.
func (or *ObsReport) MetricsRefused(ctx context.Context, numPoints int) {
or.recordData(ctx, component.DataTypeMetrics, int64(0), int64(numPoints), int64(0), int64(0))
or.telBuilder.ProcessorRefusedMetricPoints.Add(ctx, int64(numPoints), metric.WithAttributes(or.otelAttrs...))
}

// MetricsDropped reports that the metrics were dropped.
func (or *ObsReport) MetricsDropped(ctx context.Context, numPoints int) {
or.recordData(ctx, component.DataTypeMetrics, int64(0), int64(0), int64(numPoints), int64(0))
or.telBuilder.ProcessorDroppedMetricPoints.Add(ctx, int64(numPoints), metric.WithAttributes(or.otelAttrs...))
}

// MetricsInserted reports that the metrics were inserted.
func (or *ObsReport) MetricsInserted(ctx context.Context, numPoints int) {
or.recordData(ctx, component.DataTypeMetrics, int64(0), int64(0), int64(0), int64(numPoints))
or.telBuilder.ProcessorInsertedMetricPoints.Add(ctx, int64(numPoints), metric.WithAttributes(or.otelAttrs...))
}

// LogsAccepted reports that the logs were accepted.
func (or *ObsReport) LogsAccepted(ctx context.Context, numRecords int) {
or.recordData(ctx, component.DataTypeLogs, int64(numRecords), int64(0), int64(0), int64(0))
or.telBuilder.ProcessorAcceptedLogRecords.Add(ctx, int64(numRecords), metric.WithAttributes(or.otelAttrs...))
}

// LogsRefused reports that the logs were refused.
func (or *ObsReport) LogsRefused(ctx context.Context, numRecords int) {
or.recordData(ctx, component.DataTypeLogs, int64(0), int64(numRecords), int64(0), int64(0))
or.telBuilder.ProcessorRefusedLogRecords.Add(ctx, int64(numRecords), metric.WithAttributes(or.otelAttrs...))
}

// LogsDropped reports that the logs were dropped.
func (or *ObsReport) LogsDropped(ctx context.Context, numRecords int) {
or.recordData(ctx, component.DataTypeLogs, int64(0), int64(0), int64(numRecords), int64(0))
or.telBuilder.ProcessorDroppedLogRecords.Add(ctx, int64(numRecords), metric.WithAttributes(or.otelAttrs...))
}

// LogsInserted reports that the logs were inserted.
func (or *ObsReport) LogsInserted(ctx context.Context, numRecords int) {
or.recordData(ctx, component.DataTypeLogs, int64(0), int64(0), int64(0), int64(numRecords))
or.telBuilder.ProcessorInsertedLogRecords.Add(ctx, int64(numRecords), metric.WithAttributes(or.otelAttrs...))
}

0 comments on commit 49339e8

Please sign in to comment.