Skip to content

Commit

Permalink
feat(export): allow GMP collectors to startup without metadata
Browse files Browse the repository at this point in the history
  • Loading branch information
TheSpiritXIII committed May 20, 2024
1 parent a5bbe3c commit 94bb110
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 26 deletions.
1 change: 1 addition & 0 deletions .conform.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ policies:
- configs
- deps
- e2e
- export
- main
- operator
- prometheus
Expand Down
9 changes: 5 additions & 4 deletions cmd/rule-evaluator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,16 @@ func main() {
os.Exit(2)
}

exporter, err := newExporter(logger, reg)
ctxExporter := context.Background()
exporter, err := newExporter(ctxExporter, logger, reg)
if err != nil {
//nolint:errcheck
level.Error(logger).Log("msg", "Creating a Cloud Monitoring Exporter failed", "err", err)
os.Exit(1)
}
destination := export.NewStorage(exporter)

ctxRuleManger := context.Background()
ctxRuleManager := context.Background()
ctxDiscover, cancelDiscover := context.WithCancel(context.Background())

opts := []option.ClientOption{
Expand All @@ -195,7 +196,7 @@ func main() {
option.WithGRPCDialOption(grpc.WithTransportCredentials(insecure.NewCredentials())),
)
}
transport, err := apihttp.NewTransport(ctxRuleManger, http.DefaultTransport, opts...)
transport, err := apihttp.NewTransport(ctxRuleManager, http.DefaultTransport, opts...)
if err != nil {
//nolint:errcheck
level.Error(logger).Log("msg", "Creating proxy HTTP transport failed", "err", err)
Expand Down Expand Up @@ -239,7 +240,7 @@ func main() {
ruleManager := rules.NewManager(&rules.ManagerOptions{
ExternalURL: generatorURL,
QueryFunc: queryFunc,
Context: ctxRuleManger,
Context: ctxRuleManager,
Appendable: destination,
Queryable: externalStorage,
Logger: logger,
Expand Down
41 changes: 25 additions & 16 deletions pkg/export/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ func newMetricClient(ctx context.Context, opts ExporterOpts) (*monitoring.Metric
}

// New returns a new Cloud Monitoring Exporter.
func New(logger log.Logger, reg prometheus.Registerer, opts ExporterOpts) (*Exporter, error) {
func New(ctx context.Context, logger log.Logger, reg prometheus.Registerer, opts ExporterOpts) (*Exporter, error) {
grpc_prometheus.EnableClientHandlingTimeHistogram(
grpc_prometheus.WithHistogramBuckets([]float64{0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10, 15, 20, 30, 40, 50, 60}),
)
Expand Down Expand Up @@ -354,7 +354,7 @@ func New(logger log.Logger, reg prometheus.Registerer, opts ExporterOpts) (*Expo
opts.Lease = alwaysLease{}
}

metricClient, err := newMetricClient(context.Background(), opts)
metricClient, err := newMetricClient(ctx, opts)
if err != nil {
return nil, fmt.Errorf("create metric client: %w", err)
}
Expand Down Expand Up @@ -408,21 +408,30 @@ func (e *Exporter) ApplyConfig(cfg *config.Config) (err error) {
}
lset := builder.Labels()

// At this point we expect location and project ID to be set. They are effectively only a default
// however as they may be overridden by metric labels.
// In production scenarios, "location" should most likely never be overridden as it means crossing
// failure domains. Instead, each location should run a replica of the evaluator with the same rules.
if lset.Get(KeyProjectID) == "" {
return fmt.Errorf("no label %q set via external labels or flag", KeyProjectID)
}
if loc := lset.Get(KeyLocation); loc == "" {
return fmt.Errorf("no label %q set via external labels or flag", KeyLocation)
} else if loc == "global" {
return ErrLocationGlobal
}
if labels.Equal(e.externalLabels, lset) {
return nil
// We don't need to validate if there's no scrape configs or rules, i.e. at startup.
hasScrapeConfigs := len(cfg.ScrapeConfigs) != 0 || len(cfg.ScrapeConfigFiles) != 0
hasRules := len(cfg.RuleFiles) != 0
if hasScrapeConfigs || hasRules {
// At this point we expect location and project ID to be set. They are effectively
// only a default however as they may be overridden by metric labels.
//
// In production scenarios, "location" should most likely never be overridden as it
// means crossing failure domains. Instead, each location should run a replica of
// the evaluator with the same rules.

if lset.Get(KeyProjectID) == "" {
return fmt.Errorf("no label %q set via external labels or flag", KeyProjectID)
}
if loc := lset.Get(KeyLocation); loc == "" {
return fmt.Errorf("no label %q set via external labels or flag", KeyLocation)
} else if loc == "global" {
return ErrLocationGlobal
}
if labels.Equal(e.externalLabels, lset) {
return nil
}
}

// New external labels possibly invalidate the cached series conversions.
e.mtx.Lock()
e.externalLabels = lset
Expand Down
5 changes: 3 additions & 2 deletions pkg/export/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,8 @@ func TestExporter_wrapMetadata(t *testing.T) {
},
}

e, err := New(log.NewJSONLogger(log.NewSyncWriter(os.Stderr)), nil, ExporterOpts{DisableAuth: true})
ctx := context.Background()
e, err := New(ctx, log.NewJSONLogger(log.NewSyncWriter(os.Stderr)), nil, ExporterOpts{DisableAuth: true})
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -363,7 +364,7 @@ func TestExporter_drainBacklog(t *testing.T) {
t.Fatalf("Creating metric client failed: %s", err)
}

e, err := New(log.NewJSONLogger(log.NewSyncWriter(os.Stderr)), nil, ExporterOpts{DisableAuth: true})
e, err := New(ctx, log.NewJSONLogger(log.NewSyncWriter(os.Stderr)), nil, ExporterOpts{DisableAuth: true})
if err != nil {
t.Fatalf("Creating Exporter failed: %s", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/export/gcm/promtest/local_export.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (l *localExportWithGCM) start(t testing.TB, _ e2e.Environment) (v1.API, map
t.Fatalf("create Prometheus client: %s", err)
}

l.e, err = export.New(log.NewJSONLogger(os.Stderr), prometheus.NewRegistry(), export.ExporterOpts{
l.e, err = export.New(ctx, log.NewJSONLogger(os.Stderr), prometheus.NewRegistry(), export.ExporterOpts{
UserAgentEnv: "pe-github-action-test",
Endpoint: "monitoring.googleapis.com:443",
Compression: "none",
Expand Down
7 changes: 4 additions & 3 deletions pkg/export/setup/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package setup

import (
"context"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -89,7 +90,7 @@ func Global() *export.Exporter {
// FromFlags returns a constructor for a new exporter that is configured through flags that are
// registered with the given application. The constructor must be called after the flags
// have been parsed.
func FromFlags(a *kingpin.Application, userAgentProduct string) func(log.Logger, prometheus.Registerer) (*export.Exporter, error) {
func FromFlags(a *kingpin.Application, userAgentProduct string) func(context.Context, log.Logger, prometheus.Registerer) (*export.Exporter, error) {
var opts export.ExporterOpts
env := UAEnvUnspecified

Expand Down Expand Up @@ -183,7 +184,7 @@ func FromFlags(a *kingpin.Application, userAgentProduct string) func(log.Logger,
kubeName := a.Flag("export.ha.kube.name", "Name for the HA locking resource. Must be identical across replicas. May be set through the KUBE_NAME environment variable.").
Default("").OverrideDefaultFromEnvar("KUBE_NAME").String()

return func(logger log.Logger, metrics prometheus.Registerer) (*export.Exporter, error) {
return func(ctx context.Context, logger log.Logger, metrics prometheus.Registerer) (*export.Exporter, error) {
switch *haBackend {
case HABackendNone:
case HABackendKubernetes:
Expand All @@ -204,7 +205,7 @@ func FromFlags(a *kingpin.Application, userAgentProduct string) func(log.Logger,
default:
return nil, fmt.Errorf("unexpected HA backend %q", *haBackend)
}
return export.New(logger, metrics, opts)
return export.New(ctx, logger, metrics, opts)
}
}

Expand Down

0 comments on commit 94bb110

Please sign in to comment.