Add metric metadata syncer to SignalFx exporter #231

asuresh4 · 2020-05-12T16:59:41Z

Description: Added ability to sync properties and tags in SignalFx exporter. To begin with the exporter will be able to handle metadata updates from k8sclusterreceiver. The dimension updates are however handled in a generic manner to support similar metadata from other receivers. The dimension client used to perform the metric metadata sync is mostly a 1:1 port of https://github.com/signalfx/signalfx-agent/blob/master/pkg/core/writer/dimensions/client.go, with the primary difference being that deduplication of dimension updates are not currently supported in this exporter.

Link to tracking Issue: #180

Testing: Added unit tests and manual testing.

codecov-io · 2020-05-12T22:09:38Z

Codecov Report

❗ No coverage uploaded for pull request base (master@a1020fa). Click here to learn what that means.
The diff coverage is 81.45%.

@@            Coverage Diff            @@
##             master     #231   +/-   ##
=========================================
  Coverage          ?   78.16%           
=========================================
  Files             ?      130           
  Lines             ?     6828           
  Branches          ?        0           
=========================================
  Hits              ?     5337           
  Misses            ?     1194           
  Partials          ?      297

Impacted Files	Coverage Δ
.../signalfxexporter/dimensions/kubernetesmetadata.go	`65.95% <65.95%> (ø)`
...ter/signalfxexporter/dimensions/dimensionupdate.go	`75.00% <75.00%> (ø)`
exporter/signalfxexporter/dimensions/dimclient.go	`82.69% <82.69%> (ø)`
exporter/signalfxexporter/dimensions/requests.go	`84.48% <84.48%> (ø)`
exporter/signalfxexporter/config.go	`81.39% <88.23%> (ø)`
exporter/signalfxexporter/exporter.go	`86.53% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1020fa...563685d. Read the comment docs.

jrcamp · 2020-05-13T15:01:13Z

exporter/signalfxexporter/dimensions/dimclient.go

+	// For easier unit testing
+	now func() time.Time
+
+	// TODO: Send these counters as internal metrics to SignalFx


Ideally we could expose these through obsreport or some central mechanism. Maybe also TODO to add tracing support?

Tracing support as in collect spans from the DimensionClient?

Sorry, no I mean just self-instrumentation with traces through obsreport.

bogdandrutu · 2020-05-14T19:22:12Z

This PR depends on #214, will rebase once that's merged.

Is that done? If yes please update description.

asuresh4 · 2020-05-14T21:31:57Z

I just realized I haven't handled metadata updates to the same dimension within a small amount of time gracefully. Will get an update in for that shortly.

exporter/signalfxexporter/dimensions/kubernetesmetadata.go

exporter/signalfxexporter/dimensions/dimclient.go

dmitryax

Looks like dimclient.go is not covered well with tests here. I see that there is client_test.go in signalfx repo. Do we plan to add them later?

exporter/signalfxexporter/exporter_test.go

asuresh4 · 2020-05-21T17:03:52Z

Looks like dimclient.go is not covered well with tests here. I see that there is client_test.go in signalfx repo. Do we plan to add them later?

Will add those tests as well.

dmitryax

Overall looks good to me

exporter/signalfxexporter/exporter.go

exporter/signalfxexporter/dimensions/dimclient.go

dmitryax · 2020-05-22T04:35:04Z

exporter/signalfxexporter/dimensions/dimclient.go

+}
+
+// setPropertiesOnDimension will set custom properties on a specific dimension value.
+func (dc *DimensionClient) setPropertiesOnDimension(dimUpdate *DimensionUpdate) error {


The name sounds a bit confusing. May be something like sendDimensionUpdate or dispatchDimensionUpdate? What do you think?

Renamed the method handleDimensionUpdate. Since the actual sending/dispatching is done by the method called makePatchRequest.

dmitryax · 2020-05-22T04:49:30Z

exporter/signalfxexporter/dimensions/dimclient.go

+		err error
+	)
+
+	if dimUpdate.Name == "" || dimUpdate.Value == "" {


should we check this earlier? in acceptDimension or even in PushKubernetesMetadata?

Moved it for now since it seems to make more sense. When the export supports metadata updates from other sources as well, we may want to refactor.

dmitryax

LGTM

tigrannajaryan · 2020-05-22T20:26:54Z

@asuresh4 Please resolve the conflicts.

asuresh4 · 2020-05-26T11:43:06Z

@asuresh4 Please resolve the conflicts.

Done.

Added ability to sync properties and tags in SignalFx exporter. To begin with the exporter will be able to handle metadata updates from `k8sclusterreceiver`. The dimension updates are however handled in a generic manner to support similar metadata from other receivers. The dimension client used to perform the metric metadata sync is mostly a 1:1 port of https://github.com/signalfx/signalfx-agent/blob/master/pkg/core/writer/dimensions/client.go, with the primary difference being that deduplication of dimension updates are not currently supported in this exporter. **Link to tracking Issue:** open-telemetry#180 **Testing:** Added unit tests and manual testing.

* Support metrics exporter with oc exporter config Also remove outdated v1 configs and fix tests. Updates #231. * Fix static check * Add documentation * Send proto metrics with ExportMetricsServiceRequest() Fixes #231. * Remove config TOC * Add document to config.go

Bumps [go.opentelemetry.io/collector](https://github.com/open-telemetry/opentelemetry-collector) from 0.30.1 to 0.31.0. - [Release notes](https://github.com/open-telemetry/opentelemetry-collector/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-collector/blob/main/CHANGELOG.md) - [Commits](open-telemetry/opentelemetry-collector@v0.30.1...v0.31.0) --- updated-dependencies: - dependency-name: go.opentelemetry.io/collector dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

asuresh4 requested a review from a team as a code owner May 12, 2020 16:59

asuresh4 marked this pull request as draft May 12, 2020 17:00

asuresh4 marked this pull request as ready for review May 12, 2020 22:54

jrcamp approved these changes May 13, 2020

View reviewed changes

asuresh4 added 8 commits May 18, 2020 13:42

signalfxreceiver: Add k8s metric metadata syncing

6d47cca

Fix tests

dea70f1

Add unit tests

0d42a96

Fix load test

6fc8156

Actually fix load test

57b63c7

Add few more tests

53f0612

Gracefully handle quick successive updates

0093037

Merge branch 'master' into sfxexporter-k8s

aada8a7

tigrannajaryan added this to In progress in Collector via automation May 19, 2020

tigrannajaryan assigned dmitryax May 20, 2020

dmitryax reviewed May 21, 2020

View reviewed changes

exporter/signalfxexporter/dimensions/kubernetesmetadata.go Outdated Show resolved Hide resolved

exporter/signalfxexporter/dimensions/dimclient.go Outdated Show resolved Hide resolved

Address feedback

1db0c42

asuresh4 requested a review from dmitryax May 21, 2020 16:31

dmitryax reviewed May 21, 2020

View reviewed changes

exporter/signalfxexporter/exporter_test.go Show resolved Hide resolved

Synchronize test to ensure assertion

1966224

asuresh4 added 3 commits May 21, 2020 15:52

Improve test coverage

5fe2244

Merge branch 'master' into sfxexporter-k8s

fa140d1

Fix race in test

596dc74

asuresh4 requested a review from dmitryax May 21, 2020 20:12

dmitryax reviewed May 22, 2020

View reviewed changes

Address next round of feedback

351801f

Improve README

58eba07

asuresh4 requested a review from dmitryax May 22, 2020 17:13

Collector automation moved this from In progress to Reviewer approved May 22, 2020

dmitryax approved these changes May 22, 2020

View reviewed changes

Merge branch 'master' into sfxexporter-k8s

563685d

tigrannajaryan merged commit 11a5dad into open-telemetry:master May 26, 2020

Collector automation moved this from Reviewer approved to Done May 26, 2020

asuresh4 deleted the sfxexporter-k8s branch May 26, 2020 16:24

asuresh4 mentioned this pull request May 28, 2020

REQUEST: New membership for asuresh4 open-telemetry/community#358

Closed

6 tasks

ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022

Add span data race tests to tracer test harness (#231)

918efca

codeboten pushed a commit that referenced this pull request Nov 23, 2022

Add test case for DataDog fields and AWS SDK Extension (#231)

e5a0153

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metric metadata syncer to SignalFx exporter #231

Add metric metadata syncer to SignalFx exporter #231

asuresh4 commented May 12, 2020 •

edited

codecov-io commented May 12, 2020 •

edited by codecov bot

jrcamp May 13, 2020

asuresh4 May 13, 2020

jrcamp May 13, 2020

bogdandrutu commented May 14, 2020

asuresh4 commented May 14, 2020

dmitryax left a comment •

edited

asuresh4 commented May 21, 2020

dmitryax left a comment

dmitryax May 22, 2020

asuresh4 May 22, 2020

dmitryax May 22, 2020

asuresh4 May 22, 2020

dmitryax left a comment

tigrannajaryan commented May 22, 2020

asuresh4 commented May 26, 2020

Add metric metadata syncer to SignalFx exporter #231

Add metric metadata syncer to SignalFx exporter #231

Conversation

asuresh4 commented May 12, 2020 • edited

codecov-io commented May 12, 2020 • edited by codecov bot

Codecov Report

jrcamp May 13, 2020

Choose a reason for hiding this comment

asuresh4 May 13, 2020

Choose a reason for hiding this comment

jrcamp May 13, 2020

Choose a reason for hiding this comment

bogdandrutu commented May 14, 2020

asuresh4 commented May 14, 2020

dmitryax left a comment • edited

Choose a reason for hiding this comment

asuresh4 commented May 21, 2020

dmitryax left a comment

Choose a reason for hiding this comment

dmitryax May 22, 2020

Choose a reason for hiding this comment

asuresh4 May 22, 2020

Choose a reason for hiding this comment

dmitryax May 22, 2020

Choose a reason for hiding this comment

asuresh4 May 22, 2020

Choose a reason for hiding this comment

dmitryax left a comment

Choose a reason for hiding this comment

tigrannajaryan commented May 22, 2020

asuresh4 commented May 26, 2020

asuresh4 commented May 12, 2020 •

edited

codecov-io commented May 12, 2020 •

edited by codecov bot

dmitryax left a comment •

edited