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

Instrument instances types #1484

Merged
merged 18 commits into from
Jul 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,22 @@ require (
github.com/opentracing/opentracing-go v1.1.0
github.com/operator-framework/operator-sdk v0.18.2
github.com/pkg/errors v0.9.1
github.com/prometheus/common v0.9.1
github.com/sirupsen/logrus v1.5.0
github.com/prometheus/client_golang v1.10.0
github.com/prometheus/common v0.18.0
github.com/sirupsen/logrus v1.6.0
github.com/spf13/cobra v1.0.0
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.7.1
github.com/stretchr/testify v1.7.0
github.com/uber/jaeger-client-go v2.20.1+incompatible
go.opentelemetry.io/otel v0.20.0
go.opentelemetry.io/otel/exporters/metric/prometheus v0.20.0
go.opentelemetry.io/otel/exporters/trace/jaeger v0.20.0
go.opentelemetry.io/otel/metric v0.20.0
go.opentelemetry.io/otel/oteltest v0.20.0
go.opentelemetry.io/otel/sdk v0.20.0
go.opentelemetry.io/otel/sdk/export/metric v0.20.0
go.opentelemetry.io/otel/sdk/metric v0.20.0
go.opentelemetry.io/otel/trace v0.20.0
google.golang.org/grpc v1.32.0 // indirect
k8s.io/api v0.20.7
Expand Down
109 changes: 89 additions & 20 deletions go.sum

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion pkg/cmd/start/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/jaegertracing/jaeger-operator/pkg/apis"
v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
"github.com/jaegertracing/jaeger-operator/pkg/controller"
opmetrics "github.com/jaegertracing/jaeger-operator/pkg/metrics"
"github.com/jaegertracing/jaeger-operator/pkg/tracing"
"github.com/jaegertracing/jaeger-operator/pkg/upgrade"
"github.com/jaegertracing/jaeger-operator/pkg/version"
Expand Down Expand Up @@ -88,7 +89,10 @@ func bootstrap(ctx context.Context) manager.Manager {
serveCRMetrics(ctx, cfg, namespace)
createMetricsService(ctx, cfg, namespace)
detectOAuthProxyImageStream(ctx, mgr)

err = opmetrics.Bootstrap(ctx, namespace, mgr.GetClient())
if err != nil {
log.WithError(err).Error("failed to initialize metrics")
}
return mgr
}

Expand Down
57 changes: 57 additions & 0 deletions pkg/metrics/bootstrap.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package metrics

import (
"context"

prometheusclient "github.com/prometheus/client_golang/prometheus"
rubenvp8510 marked this conversation as resolved.
Show resolved Hide resolved
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/exporters/metric/prometheus"
"go.opentelemetry.io/otel/metric/global"
export "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregator/histogram"
controller "go.opentelemetry.io/otel/sdk/metric/controller/basic"
processor "go.opentelemetry.io/otel/sdk/metric/processor/basic"
selector "go.opentelemetry.io/otel/sdk/metric/selector/simple"
"go.opentelemetry.io/otel/sdk/resource"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/metrics"

"github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
"github.com/jaegertracing/jaeger-operator/pkg/tracing"
)

const meterName = "jaegertracing.io/jaeger"

// Bootstrap configures the OpenTelemetry meter provider with the Prometheus exporter.
func Bootstrap(ctx context.Context, namespace string, client client.Client) error {
tracer := otel.GetTracerProvider().Tracer(v1.BootstrapTracer)
ctx, span := tracer.Start(ctx, "bootstrap")
Copy link
Member

Choose a reason for hiding this comment

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

is this span recording events/ reported?

If the bootstrap creates tracer than the span might be noop/default if it is created before initialization.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The tracer is already created when this bootstraping is called, this bootstraps the metrics for Open Telemetry,

defer span.End()
tracing.SetInstanceID(ctx, namespace)
jpkrohling marked this conversation as resolved.
Show resolved Hide resolved

config := prometheus.Config{
Registry: metrics.Registry.(*prometheusclient.Registry),
}
c := controller.New(
processor.New(
selector.NewWithHistogramDistribution(
histogram.WithExplicitBoundaries(config.DefaultHistogramBoundaries),
Copy link
Contributor

Choose a reason for hiding this comment

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

What are the boundaries defined here? Do they make sense for us?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

None of that is applicable to us. for now. That is why I just set default values.

Copy link
Contributor

Choose a reason for hiding this comment

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

What are the default values? If we don't have histograms, why do we configure them? Can't we just leave this out?

Copy link
Collaborator Author

@rubenvp8510 rubenvp8510 Jul 13, 2021

Choose a reason for hiding this comment

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

What are the default values? If we don't have histograms, why do we configure them? Can't we just leave this out?

I don't know, we are not using it. I can't I need to pass and aggregation selector to the New method. :/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

// NewWithHistogramDistribution returns a simple aggregator selector
// that uses histogram aggregators for `ValueRecorder` instruments.
// This selector is a good default choice for most metric exporters.

From this comment I'm assuming this is the best "default" configuration.

),
export.CumulativeExportKindSelector(),
processor.WithMemory(true),
),
controller.WithResource(resource.NewWithAttributes([]attribute.KeyValue{}...)),
)
exporter, err := prometheus.NewExporter(config, c)
if err != nil {
rubenvp8510 marked this conversation as resolved.
Show resolved Hide resolved
return tracing.HandleError(err, span)
}

global.SetMeterProvider(exporter.MeterProvider())

// Create metrics
instancesObservedValue := newInstancesMetric(client)
err = instancesObservedValue.Setup(ctx)
return tracing.HandleError(err, span)
}
160 changes: 160 additions & 0 deletions pkg/metrics/instances.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package metrics

import (
"context"
"fmt"
"strings"

"go.opentelemetry.io/otel"
rubenvp8510 marked this conversation as resolved.
Show resolved Hide resolved
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/global"
"sigs.k8s.io/controller-runtime/pkg/client"

v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
)

const metricPrefix = "jaeger_operator_instances"
rubenvp8510 marked this conversation as resolved.
Show resolved Hide resolved
const agentStrategiesMetric = "agent_strategies"
const storageMetric = "storage_types"
const strategiesMetric = "strategies"

// This structure contains the labels associated with the instances and a counter of the number of instances
type instancesView struct {
Name string
Label string
Count map[string]int
Observer *metric.Int64ValueObserver
KeyFn func(jaeger v1.Jaeger) string
}

func (i *instancesView) reset() {
for k := range i.Count {
i.Count[k] = 0
}
}

func (i *instancesView) Record(jaeger v1.Jaeger) {
i.Count[i.KeyFn(jaeger)]++
}

func (i *instancesView) Report(result metric.BatchObserverResult) {
for key, count := range i.Count {
result.Observe([]attribute.KeyValue{
attribute.String(i.Label, key),
}, i.Observer.Observation(int64(count)))
}
}

type instancesMetric struct {
client client.Client
observations []instancesView
}

func instanceMetricName(name string) string {
return fmt.Sprintf("%s_%s", metricPrefix, name)
}

func newInstancesMetric(client client.Client) *instancesMetric {
return &instancesMetric{
client: client,
}
}

func newObservation(batch metric.BatchObserver, name, desc, label string, keyFn func(jaeger v1.Jaeger) string) (instancesView, error) {
observation := instancesView{
Name: name,
Count: make(map[string]int),
KeyFn: keyFn,
Label: label,
}
obs, err := batch.NewInt64ValueObserver(instanceMetricName(name), metric.WithDescription(desc))
if err != nil {
return instancesView{}, err
}
observation.Observer = &obs
return observation, nil
}

func (i *instancesMetric) Setup(ctx context.Context) error {
tracer := otel.GetTracerProvider().Tracer(v1.BootstrapTracer)
ctx, span := tracer.Start(ctx, "setup-jaeger-instances-metrics")
defer span.End()
meter := global.Meter(meterName)
batch := meter.NewBatchObserver(i.callback)
obs, err := newObservation(batch,
agentStrategiesMetric,
"Number of instances per agent strategy",
"type",
func(jaeger v1.Jaeger) string {
return strings.ToLower(string(jaeger.Spec.Agent.Strategy))
})

if err != nil {
return err
}
i.observations = append(i.observations, obs)
obs, err = newObservation(batch, storageMetric,
"Number of instances per storage type",
"type",
func(jaeger v1.Jaeger) string {
return strings.ToLower(string(jaeger.Spec.Storage.Type))
})
if err != nil {
return err
}
i.observations = append(i.observations, obs)

obs, err = newObservation(batch, strategiesMetric,
"Number of instances per strategy type",
"type",
func(jaeger v1.Jaeger) string {
return strings.ToLower(string(jaeger.Spec.Strategy))
})
if err != nil {
return err
}
i.observations = append(i.observations, obs)
return nil
}

func isInstanceNormalized(jaeger v1.Jaeger) bool {
return !(jaeger.Spec.Storage.Type == "" || jaeger.Spec.Strategy == "")
rubenvp8510 marked this conversation as resolved.
Show resolved Hide resolved
}

func normalizeAgentStrategy(jaeger *v1.Jaeger) {
if jaeger.Spec.Agent.Strategy == "" {
jaeger.Spec.Agent.Strategy = "Sidecar"
}
}

func (i *instancesMetric) reset() {
for _, o := range i.observations {
o.reset()
}
}

func (i *instancesMetric) report(result metric.BatchObserverResult) {
for _, o := range i.observations {
o.Report(result)
}
}

func (i *instancesMetric) callback(ctx context.Context, result metric.BatchObserverResult) {
instances := &v1.JaegerList{}
if err := i.client.List(ctx, instances); err == nil {
i.reset()
for _, jaeger := range instances.Items {
// Is this instance is already normalized by the reconciliation process
// count it on the metrics, otherwise not.
if isInstanceNormalized(jaeger) {
// Normalization doesn't normalize agent mode. so we need to do it here.
normalizeAgentStrategy(&jaeger)
for _, o := range i.observations {
o.Record(jaeger)
}
}
}
i.report(result)
}
}
Loading