Skip to content
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

feat: Add support to emit Metrics same as Traces via OTEL #456

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Zurvarian
Copy link
Contributor

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind fix
/kind build
/kind chore
/kind ci
/kind docs
/kind style
/kind refactor
/kind perf
/kind test

/kind feat

What this PR does / Why we need it:
Add support to emit metrics (GRPC, Redis, etc..) via OTLP.

Which issue(s) this PR fixes:

Closes #455

Special notes for your reviewer:
None

var tracer *trace.TracerProvider
if cfg.EnableTrace {
log.Printf("Enabling CloudTrace exporter with sample rate: %f\n", cfg.ServerConfig.TraceSampleRate)

grpcOptions = append(grpcOptions,
grpc.UnaryInterceptor(otelgrpc.UnaryServerInterceptor()),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UnaryServerInterceptor and StreamServerInterceptor have been deprecated in favour of NewServerHandler()

EnableTrace: viper.GetBool(EnableTrace),
TraceSampleRate: viper.GetFloat64(TraceSampleRate),
TraceServiceName: viper.GetString(TraceServiceName),
EnableGRPCCollector: viper.GetBool(TraceEnableGRPCCollector),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the Trace prefix for both EnableGRPCCollector and EnableHTTPCollector to differentiate them from the counter parts of Metrics.

It is important to mention that the OTLP endpoint for Traces and Metrics could or could not be the same.

if err != nil {
return nil, err
}
options = append(options, metric.WithReader(metric.NewPeriodicReader(exporter)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are options we can configure at the PeriodicReader, like the frequency, but I didn't want to make configuration too complex, and the defaults are sensible.

@vasconcelosvcd
Copy link
Collaborator

/gcbrun

1 similar comment
@Zurvarian
Copy link
Contributor Author

/gcbrun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to publish metrics via OTLP
3 participants