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

Upgrade telemetry sdk #36

Merged
merged 5 commits into from
Feb 5, 2020
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
5 changes: 0 additions & 5 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@
*.log
tmp/
vendor/*/
!vendor/github.com/
vendor/github.com/*/
!vendor/github.com/newrelic/
vendor/github.com/newrelic/*/
!vendor/github.com/newrelic/go-telemetry-sdk/
bin/
deploy/local*.yaml
dist/
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased
### Changed
- Upgrade the Go Telemetry SDK to version 0.2.0 to fix an issue with NaN values.

## 1.2.2
### Added
- Obfuscate the license key when logging the configuration. Running on debug
Expand Down
10 changes: 8 additions & 2 deletions internal/cmd/scraper/scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import (
"os"
"time"

"github.com/newrelic/go-telemetry-sdk/telemetry"
"github.com/newrelic/newrelic-telemetry-sdk-go/telemetry"
"github.com/newrelic/nri-prometheus/internal/integration"
"github.com/newrelic/nri-prometheus/internal/pkg/endpoints"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus/promhttp"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -244,7 +245,12 @@ func Run(cfg *Config) error {
Percentiles: cfg.Percentiles,
HarvesterOpts: harvesterOpts,
}
emitters = append(emitters, integration.NewTelemetryEmitter(c))

emitter, err := integration.NewTelemetryEmitter(c)
if err != nil {
return errors.Wrap(err, "could not create new TelemetryEmitter")
}
emitters = append(emitters, emitter)
default:
logrus.Debugf("unknown emitter: %s", e)
continue
Expand Down
52 changes: 15 additions & 37 deletions internal/integration/emitter.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import (
"net/url"
"time"

"github.com/newrelic/go-telemetry-sdk/cumulative"
"github.com/newrelic/go-telemetry-sdk/telemetry"
"github.com/newrelic/newrelic-telemetry-sdk-go/cumulative"
"github.com/newrelic/newrelic-telemetry-sdk-go/telemetry"
"github.com/newrelic/nri-prometheus/internal/histogram"
"github.com/pkg/errors"
dto "github.com/prometheus/client_model/go"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -143,7 +144,7 @@ func TelemetryHarvesterWithProxy(proxyURL *url.URL) TelemetryHarvesterOpt {
}

// NewTelemetryEmitter returns a new TelemetryEmitter.
func NewTelemetryEmitter(cfg TelemetryEmitterConfig) *TelemetryEmitter {
func NewTelemetryEmitter(cfg TelemetryEmitterConfig) (*TelemetryEmitter, error) {
dc := cumulative.NewDeltaCalculator()

if cfg.DeltaExpirationAge != 0 {
Expand All @@ -166,12 +167,17 @@ func NewTelemetryEmitter(cfg TelemetryEmitterConfig) *TelemetryEmitter {
cfg.DeltaExpirationAge,
)

harvester, err := telemetry.NewHarvester(cfg.HarvesterOpts...)
if err != nil {
return nil, errors.Wrap(err, "could not create new Harvester")
}

return &TelemetryEmitter{
name: "telemetry",
harvester: telemetry.NewHarvester(cfg.HarvesterOpts...),
harvester: harvester,
percentiles: cfg.Percentiles,
deltaCalculator: dc,
}
}, nil
}

// Name returns the emitter name.
Expand Down Expand Up @@ -261,23 +267,12 @@ func (te *TelemetryEmitter) emitSummary(metric Metric, timestamp time.Time) erro
continue
}

v := q.GetValue()
if !validNRValue(v) {
err := fmt.Errorf("invalid percentile value for %s: %g", metric.name, v)
if results == nil {
results = err
} else {
results = fmt.Errorf("%v: %w", err, results)
}
continue
}

percentileAttrs := copyAttrs(metric.attributes)
percentileAttrs["percentile"] = p
te.harvester.RecordMetric(telemetry.Gauge{
Name: metricName,
Attributes: percentileAttrs,
Value: v,
Value: q.GetValue(),
Timestamp: timestamp,
})
}
Expand All @@ -294,18 +289,16 @@ func (te *TelemetryEmitter) emitHistogram(metric Metric, timestamp time.Time) er
return fmt.Errorf("unknown histogram metric type for %q: %T", metric.name, metric.value)
}

if validNRValue(hist.GetSampleSum()) {
if m, ok := te.deltaCalculator.CountMetric(metric.name+".sum", metric.attributes, hist.GetSampleSum(), timestamp); ok {
te.harvester.RecordMetric(m)
}
if m, ok := te.deltaCalculator.CountMetric(metric.name+".sum", metric.attributes, hist.GetSampleSum(), timestamp); ok {
te.harvester.RecordMetric(m)
}

metricName := metric.name + ".buckets"
buckets := make(histogram.Buckets, 0, len(hist.Bucket))
for _, b := range hist.GetBucket() {
upperBound := b.GetUpperBound()
count := float64(b.GetCumulativeCount())
if !math.IsInf(upperBound, 1) && validNRValue(count) {
if !math.IsInf(upperBound, 1) {
bucketAttrs := copyAttrs(metric.attributes)
bucketAttrs["histogram.bucket.upperBound"] = upperBound
if m, ok := te.deltaCalculator.CountMetric(metricName, bucketAttrs, count, timestamp); ok {
Expand Down Expand Up @@ -334,16 +327,6 @@ func (te *TelemetryEmitter) emitHistogram(metric Metric, timestamp time.Time) er
continue
}

if !validNRValue(v) {
err := fmt.Errorf("invalid percentile value for %s: %g", metric.name, v)
if results == nil {
results = err
} else {
results = fmt.Errorf("%v: %w", err, results)
}
continue
}

percentileAttrs := copyAttrs(metric.attributes)
percentileAttrs["percentile"] = p
te.harvester.RecordMetric(telemetry.Gauge{
Expand All @@ -366,11 +349,6 @@ func copyAttrs(attrs map[string]interface{}) map[string]interface{} {
return duplicate
}

// validNRValue returns if v is a New Relic metric supported float64.
func validNRValue(v float64) bool {
return !math.IsInf(v, 0) && !math.IsNaN(v)
}

// StdoutEmitter emits metrics to stdout.
type StdoutEmitter struct {
name string
Expand Down
15 changes: 9 additions & 6 deletions internal/integration/emitter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package integration
import (
"bytes"
"compress/gzip"
"context"
"crypto/tls"
"encoding/json"
"fmt"
Expand All @@ -17,14 +18,14 @@ import (
"strconv"
"testing"

"github.com/newrelic/go-telemetry-sdk/telemetry"
"github.com/pkg/errors"
dto "github.com/prometheus/client_model/go"
mpb "github.com/prometheus/client_model/go"
"github.com/prometheus/common/expfmt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/newrelic/newrelic-telemetry-sdk-go/telemetry"
"github.com/newrelic/nri-prometheus/internal/pkg/labels"
"github.com/newrelic/nri-prometheus/internal/pkg/prometheus"
)
Expand Down Expand Up @@ -67,11 +68,12 @@ func BenchmarkTelemetrySDKEmitter(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
emitter := NewTelemetryEmitter(c)
emitter, err := NewTelemetryEmitter(c)
assert.NoError(b, err)
err = emitter.Emit(superMetrics)
assert.NoError(b, err)
// Need to trigger a manual harvest here otherwise the benchmark is useless.
emitter.harvester.HarvestNow()
emitter.harvester.HarvestNow(context.Background())
}
}

Expand Down Expand Up @@ -217,11 +219,12 @@ func TestTelemetryEmitterEmit(t *testing.T) {
Percentiles: []float64{50.0},
}

e := NewTelemetryEmitter(c)
e, err := NewTelemetryEmitter(c)
assert.NoError(t, err)

// Emit and force a harvest to clear.
assert.NoError(t, e.Emit(metrics))
e.harvester.HarvestNow()
e.harvester.HarvestNow(context.Background())

// Set new histogram values so counts will be non-zero.
hist2, err := newHistogram([]int64{1, 2, 10})
Expand All @@ -232,7 +235,7 @@ func TestTelemetryEmitterEmit(t *testing.T) {

// Run twice so delta counts are sent.
assert.NoError(t, e.Emit(metrics))
e.harvester.HarvestNow()
e.harvester.HarvestNow(context.Background())
purgeTimestamps(rawMetrics)

expectedMetrics := []interface{}{
Expand Down
4 changes: 2 additions & 2 deletions internal/integration/roundtripper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ type licenseKeyRoundTripper struct {
rt http.RoundTripper
}

// RoundTrip wraps the `RoundTrip` method removing the "X-Insert-Key"
// RoundTrip wraps the `RoundTrip` method removing the "Api-Key"
// replacing it with "X-License-Key".
func (t licenseKeyRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
req.Header.Del("X-Insert-Key")
req.Header.Del("Api-Key")
req.Header.Add("X-License-Key", t.licenseKey)
return t.rt.RoundTrip(req)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/integration/roundtripper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ func (m *mockedRoundTripper) RoundTrip(req *http.Request) (*http.Response, error
func TestRoundTripHeaderDecoration(t *testing.T) {
licenseKey := "myLicenseKey"
req := &http.Request{Header: make(http.Header)}
req.Header.Add("X-Insert-Key", "insertKey")
req.Header.Add("Api-Key", licenseKey)

rt := new(mockedRoundTripper)
rt.On("RoundTrip", req).Return().Run(func(args mock.Arguments) {
req := args.Get(0).(*http.Request)
assert.Equal(t, licenseKey, req.Header.Get("X-License-Key"))
assert.Equal(t, "", req.Header.Get("X-Insert-Key"))
assert.Equal(t, "", req.Header.Get("Api-Key"))
})
tr := newLicenseKeyRoundTripper(rt, licenseKey)

Expand Down

This file was deleted.

Loading