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.

Sign up for GitHub

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

Jump to bottom

Add collector version to prometheus remote write exporter user agent #47

Closed
wants to merge 23 commits into from
Closed

Add collector version to prometheus remote write exporter user agent #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
54a340c
Add collector version to prometheus remote write exporter user agent
dhruv-vora Apr 17, 2021
9a08e25
Add collector version to prometheus remote write exporter user agent
dhruv-vora Apr 17, 2021
b4c988a
erge branch 'update-prwexporter-useragent' of https://github.com/open…
dhruv-vora Apr 21, 2021
ab7f5c3
Merge branch 'main' into update-prwexporter-useragent
dhruv-vora Apr 22, 2021
c6e2c65
refined user-agent header and tests
dhruv-vora Apr 23, 2021
f8be68a
pre-computed user-agent and updated functions
dhruv-vora Apr 23, 2021
d3d1760
removed startInfo from PrwExporter struct
dhruv-vora Apr 24, 2021
14ca544
Updated User-Agent header to make use of LongName
dhruv-vora Apr 27, 2021
24b5ae4
removed usage of GitHash
dhruv-vora Apr 27, 2021
fae3de9
using BuildInfo
dhruv-vora Apr 29, 2021
083a01e
Add collector version to prometheus remote write exporter user agent
dhruv-vora Apr 17, 2021
e14aeb4
Add collector version to prometheus remote write exporter user agent
dhruv-vora Apr 17, 2021
f3bd720
refined user-agent header and tests
dhruv-vora Apr 23, 2021
bcccddf
pre-computed user-agent and updated functions
dhruv-vora Apr 23, 2021
1655ea1
removed startInfo from PrwExporter struct
dhruv-vora Apr 24, 2021
eb3355e
removed usage of GitHash
dhruv-vora Apr 27, 2021
020de15
updated to use BuildInfo and LongName
dhruv-vora May 3, 2021
f108a27
merging
dhruv-vora May 3, 2021
6efe6e5
Merge remote-tracking branch 'origin/main' into update-prwexporter-us…
Aneurysm9 May 4, 2021
01ab306
renamed variables to use new convention
dhruv-vora May 4, 2021
16b7180
Merge remote-tracking branch 'origin/main' into update-prwexporter-us…
dhruv-vora May 5, 2021
d1463d7
removed X-Prometheus-Remote-Write-Version/0.1.0 from User-Agent header
dhruv-vora May 7, 2021
44a3398
Merge branch 'main' into update-prwexporter-useragent
dhruv-vora May 12, 2021
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
35 changes: 20 additions & 15 deletions exporter/prometheusremotewriteexporter/exporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,19 @@ import (
"math"
"net/http"
"net/url"
"strings"
"sync"

"github.com/gogo/protobuf/proto"
"github.com/golang/snappy"
"github.com/prometheus/prometheus/prompb"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/consumererror"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/internal"
otlp "go.opentelemetry.io/collector/internal/data/protogen/metrics/v1"
resourcev1 "go.opentelemetry.io/collector/internal/data/protogen/resource/v1"
"go.opentelemetry.io/collector/internal/version"
)

const (
Expand All @@ -46,17 +47,18 @@ const (

// PrwExporter converts OTLP metrics to Prometheus remote write TimeSeries and sends them to a remote endpoint.
type PrwExporter struct {
namespace string
externalLabels map[string]string
endpointURL *url.URL
client *http.Client
wg *sync.WaitGroup
closeChan chan struct{}
namespace string
externalLabels map[string]string
endpointURL *url.URL
client *http.Client
wg *sync.WaitGroup
closeChan chan struct{}
userAgentHeader string
}

// NewPrwExporter initializes a new PrwExporter instance and sets fields accordingly.
// client parameter cannot be nil.
func NewPrwExporter(namespace string, endpoint string, client *http.Client, externalLabels map[string]string) (*PrwExporter, error) {
func NewPrwExporter(namespace string, endpoint string, client *http.Client, externalLabels map[string]string, buildInfo component.BuildInfo) (*PrwExporter, error) {
if client == nil {
return nil, errors.New("http client cannot be nil")
}
Expand All @@ -71,13 +73,16 @@ func NewPrwExporter(namespace string, endpoint string, client *http.Client, exte
return nil, errors.New("invalid endpoint")
}

userAgentHeader := fmt.Sprintf("%s/%s", strings.ReplaceAll(strings.ToLower(buildInfo.Description), " ", "-"), buildInfo.Version)
Copy link

Choose a reason for hiding this comment

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

This seems like an arbitrary sanitization rule. Description can include any unicode character. We should sanitize it all if we want to autogenerate something from the description. Another alternative wouild be adding a new field to BuildInfo to allow setting custom user agents.

Also don't lowercase, uppercase is valid user agent. See https://www.ietf.org/rfc/rfc2616.html#section-3.8


return &PrwExporter{
namespace: namespace,
externalLabels: sanitizedLabels,
endpointURL: endpointURL,
client: client,
wg: new(sync.WaitGroup),
closeChan: make(chan struct{}),
namespace: namespace,
externalLabels: sanitizedLabels,
endpointURL: endpointURL,
client: client,
wg: new(sync.WaitGroup),
closeChan: make(chan struct{}),
userAgentHeader: userAgentHeader,
}, nil
}

Expand Down Expand Up @@ -322,7 +327,7 @@ func (prwe *PrwExporter) execute(ctx context.Context, writeReq *prompb.WriteRequ
req.Header.Add("Content-Encoding", "snappy")
req.Header.Set("Content-Type", "application/x-protobuf")
req.Header.Set("X-Prometheus-Remote-Write-Version", "0.1.0")
req.Header.Set("User-Agent", "OpenTelemetry-Collector/"+version.Version)
req.Header.Set("User-Agent", prwe.userAgentHeader)

resp, err := prwe.client.Do(req)
if err != nil {
Expand Down
33 changes: 27 additions & 6 deletions exporter/prometheusremotewriteexporter/exporter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/consumer/pdata"
Expand All @@ -37,7 +38,6 @@ import (
otlpcollectormetrics "go.opentelemetry.io/collector/internal/data/protogen/collector/metrics/v1"
otlp "go.opentelemetry.io/collector/internal/data/protogen/metrics/v1"
"go.opentelemetry.io/collector/internal/testdata"
"go.opentelemetry.io/collector/internal/version"
)

// Test_ NewPrwExporter checks that a new exporter instance with non-nil fields is initialized
Expand All @@ -51,6 +51,11 @@ func Test_NewPrwExporter(t *testing.T) {
ExternalLabels: map[string]string{},
HTTPClientSettings: confighttp.HTTPClientSettings{Endpoint: ""},
}
buildInfo := component.BuildInfo{
Description: "OpenTelemetry Collector",
Version: "1.0",
}

tests := []struct {
name string
config *Config
Expand All @@ -59,6 +64,7 @@ func Test_NewPrwExporter(t *testing.T) {
externalLabels map[string]string
client *http.Client
returnError bool
buildInfo component.BuildInfo
}{
{
"invalid_URL",
Expand All @@ -68,6 +74,7 @@ func Test_NewPrwExporter(t *testing.T) {
map[string]string{"Key1": "Val1"},
http.DefaultClient,
true,
buildInfo,
},
{
"nil_client",
Expand All @@ -77,6 +84,7 @@ func Test_NewPrwExporter(t *testing.T) {
map[string]string{"Key1": "Val1"},
nil,
true,
buildInfo,
},
{
"invalid_labels_case",
Expand All @@ -86,6 +94,7 @@ func Test_NewPrwExporter(t *testing.T) {
map[string]string{"Key1": ""},
http.DefaultClient,
true,
buildInfo,
},
{
"success_case",
Expand All @@ -95,6 +104,7 @@ func Test_NewPrwExporter(t *testing.T) {
map[string]string{"Key1": "Val1"},
http.DefaultClient,
false,
buildInfo,
},
{
"success_case_no_labels",
Expand All @@ -104,12 +114,13 @@ func Test_NewPrwExporter(t *testing.T) {
map[string]string{},
http.DefaultClient,
false,
buildInfo,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
prwe, err := NewPrwExporter(tt.namespace, tt.endpoint, tt.client, tt.externalLabels)
prwe, err := NewPrwExporter(tt.namespace, tt.endpoint, tt.client, tt.externalLabels, tt.buildInfo)
if tt.returnError {
assert.Error(t, err)
return
Expand All @@ -121,6 +132,7 @@ func Test_NewPrwExporter(t *testing.T) {
assert.NotNil(t, prwe.client)
assert.NotNil(t, prwe.closeChan)
assert.NotNil(t, prwe.wg)
assert.NotNil(t, prwe.userAgentHeader)
})
}
}
Expand Down Expand Up @@ -168,7 +180,7 @@ func Test_export(t *testing.T) {
// Receives the http requests and unzip, unmarshals, and extracts TimeSeries
assert.Equal(t, "0.1.0", r.Header.Get("X-Prometheus-Remote-Write-Version"))
assert.Equal(t, "snappy", r.Header.Get("Content-Encoding"))
assert.Equal(t, "OpenTelemetry-Collector/"+version.Version, r.Header.Get("User-Agent"))
assert.Equal(t, "opentelemetry-collector/1.0", r.Header.Get("User-Agent"))
writeReq := &prompb.WriteRequest{}
unzipped := []byte{}

Expand Down Expand Up @@ -245,8 +257,13 @@ func runExportPipeline(ts *prompb.TimeSeries, endpoint *url.URL) []error {
testmap["test"] = ts

HTTPClient := http.DefaultClient

buildInfo := component.BuildInfo{
Description: "OpenTelemetry Collector",
Version: "1.0",
}
// after this, instantiate a CortexExporter with the current HTTP client and endpoint set to passed in endpoint
prwe, err := NewPrwExporter("test", endpoint.String(), HTTPClient, map[string]string{})
prwe, err := NewPrwExporter("test", endpoint.String(), HTTPClient, map[string]string{}, buildInfo)
if err != nil {
errs = append(errs, err)
return errs
Expand Down Expand Up @@ -507,7 +524,7 @@ func Test_PushMetrics(t *testing.T) {
dest, err := snappy.Decode(buf, body)
assert.Equal(t, "0.1.0", r.Header.Get("x-prometheus-remote-write-version"))
assert.Equal(t, "snappy", r.Header.Get("content-encoding"))
assert.Equal(t, "OpenTelemetry-Collector/"+version.Version, r.Header.Get("user-agent"))
assert.Equal(t, "opentelemetry-collector/1.0", r.Header.Get("User-Agent"))
assert.NotNil(t, r.Header.Get("tenant-id"))
require.NoError(t, err)
wr := &prompb.WriteRequest{}
Expand Down Expand Up @@ -698,7 +715,11 @@ func Test_PushMetrics(t *testing.T) {
// c, err := config.HTTPClientSettings.ToClient()
// assert.Nil(t, err)
c := http.DefaultClient
prwe, nErr := NewPrwExporter(config.Namespace, serverURL.String(), c, map[string]string{})
buildInfo := component.BuildInfo{
Description: "OpenTelemetry Collector",
Version: "1.0",
}
prwe, nErr := NewPrwExporter(config.Namespace, serverURL.String(), c, map[string]string{}, buildInfo)
require.NoError(t, nErr)
err := prwe.PushMetrics(context.Background(), *tt.md)
if tt.returnErr {
Expand Down
2 changes: 1 addition & 1 deletion exporter/prometheusremotewriteexporter/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func createMetricsExporter(_ context.Context, params component.ExporterCreatePar
return nil, err
}

prwe, err := NewPrwExporter(prwCfg.Namespace, prwCfg.HTTPClientSettings.Endpoint, client, prwCfg.ExternalLabels)
prwe, err := NewPrwExporter(prwCfg.Namespace, prwCfg.HTTPClientSettings.Endpoint, client, prwCfg.ExternalLabels, params.BuildInfo)
if err != nil {
return nil, err
}
Expand Down