Adding a trace exporter to Azure Monitor #39

pcwiese · 2019-11-19T19:19:54Z

This exporter transforms the current wire format Spans into constructs
defined in ApplicationInsights-Go.

Once a Span is transformed it is sent to Azure Monitor via a transportChannel interface. There is an implementation of that interface method inside ApplicationInsights-Go package that ultimately ultimately sends to Azure Monitor via a REST API. This package takes care of appropriate retries, batching, and buffering (in-memory only).

Once the OpenTelemetry wire format is incorporated into the collector I will make the switch to that format. The changes should not be too bad.

This PR does not:

export Span events
export Span links,
implement a metrics exporter

Test code coverage is > 90%

owais

First pass. Will take another look tomorrow.

exporter/azuremonitorexporter/config.go

exporter/azuremonitorexporter/exporter_common.go

exporter/azuremonitorexporter/traceexporter.go

pcwiese · 2019-11-21T19:21:18Z

I could use some help wiring this up correctly to the build I think. Even though the checks passed, I don't see my tests running in the build output and I see some other warnings. Makefile issues possibly...

pcwiese · 2019-11-21T22:50:47Z

I could use some help wiring this up correctly to the build I think. Even though the checks passed, I don't see my tests running in the build output and I see some other warnings. Makefile issues possibly...

Figured it out. Seems that I had to wire up the exporter in the components.go in order for the tests to run.

exporter/azuremonitorexporter/factory.go

exporter/azuremonitorexporter/factory_test.go

exporter/azuremonitorexporter/traceexporter.go

exporter/azuremonitorexporter/traceexporter_test.go

go.mod

pcwiese · 2019-11-26T05:05:07Z

This exporter transforms the current wire format Spans into constructs
defined in ApplicationInsights-Go.

The link is broken.

Fixed.

exporter/azuremonitorexporter/config_test.go

pcwiese · 2019-11-26T16:21:14Z

Is a readme.md in the root folder of the exporter a good practice? Perhaps it can say what is Azure Monitor and where to get the instrumentation key from.

I agree a README file is in order. I'll make a note to add one in a later PR.

pcwiese · 2019-11-29T21:15:31Z

Can I get an approval from one of the required reviewers?

This exporter transforms the current wire format Spans into constructs defined in [ApplicationInsights-Go](github.com/Microsoft/ApplicationInsights-Go/appinsights/contracts). Once a Span is transformed it is sent to Azure Monitor via a transportChannel interface. There is an implementation of that interface method inside ApplicationInsights-Go package that ultimately ultimately sent to Azure Monitor via a REST API. This package takes care of appropriate retries, batching, and buffering (in-memory only). Once the OpenTelemetry wire format is incorporated into the collector I will make the switch to that format. The changes should not be too bad. This PR does not: - export Span events - export Span links, - implement a metrics exporter Test code coverage is > 90%

…tion available in the zap.Logger

pjanotti

Hi @pcwiese sorry for the delay (I was on vacation). Could you please add a call to assert.NoError(t, configcheck.ValidateConfig(cfg)) to some test? It is going to pass but it is good form to have it so if any future validation is added we will be able to spot issues at the package level.

exporter/azuremonitorexporter/factory.go

go.mod

pjanotti

LGTM.

exporter/azuremonitorexporter/config.go

exporter/azuremonitorexporter/testdata/config.yaml

pjanotti · 2019-12-03T17:54:37Z

Merged @pcwiese, thanks for the PR.

* Initial commit * Add CODEOWNERS file (open-telemetry#2) * Add CODEOWNERS file * Update CODEOWNERS * Moved from github.com/observatorium/opentelemetry-collector-builder (open-telemetry#3) Signed-off-by: Juraci Paixão Kröhling <[email protected]> * fixed panics (open-telemetry#6) Signed-off-by: Joe Elliott <[email protected]> * Replace master with main in CI and mergify files (open-telemetry#8) Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Bump to OpenTelemetry Collector 0.20.0 (open-telemetry#10) Closes open-telemetry#9 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Explicitly enable Go modules in quickstart instructions (open-telemetry#13) * Update to collector v0.21.0 (open-telemetry#17) Fixes open-telemetry#16 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Update to collector v0.22.0 (open-telemetry#19) * Download go modules before building (open-telemetry#20) Fixes open-telemetry#14 * Add version command (open-telemetry#25) Signed-off-by: Ashmita Bohara <[email protected]> * Pass errors from cobra Execute back to main for correct exit code (open-telemetry#28) * pass errors from cobra execute back to main * print the error * Update to collector v0.23.0 (open-telemetry#27) * Generate a warning if the builder and collector base version mismatch (open-telemetry#30) * Generate a warning if the builder and collector base version mismatch * Show current default version in the warning message * Update to OpenTelemetry Collector 0.24.0 * Don't use %w formatting with log.Fatal (open-telemetry#35) * Update to OpenTelemetry Collector 0.25.0 (open-telemetry#36) Signed-off-by: Serge Catudal <[email protected]> * Update to 0.26.0 and update BuildInfo (open-telemetry#39) * Sync build and CI Go versions at latest 1.16 (open-telemetry#34) * Sync build and CI Go versions at latest 1.16 * Run go mod tidy * Set go binary to use in the compilation phase in tests Signed-off-by: Juraci Paixão Kröhling <[email protected]> Co-authored-by: Juraci Paixão Kröhling <[email protected]> * Add option to generate go code only (no compile) (open-telemetry#40) * Issue#24 Add option to generate go code only (no compile) * Update cmd/root.go logging Suggested by @jpkkrohling Co-authored-by: Juraci Paixão Kröhling <[email protected]> * remove verbose help .. created by corba * suggestion by jpkrohling to keep generateandcompile * lint error: remove unused var * reword cmd option and add back help message for default Co-authored-by: Juraci Paixão Kröhling <[email protected]> * Don't reuse exec.Cmd (open-telemetry#42) * Update to OpenTelemetry Collector 0.27.0 (open-telemetry#43) * Add CI Badge (open-telemetry#47) * Update to Collector v0.28.0 (open-telemetry#49) * Update to Collector v0.28.0 Closes open-telemetry#48 Addresses the breaking API change in open-telemetry/opentelemetry-collector#3163, besides the usual version number changes. Signed-off-by: Fangyi Zhou <[email protected]> * Use `go mod tidy` instead of `go mod download` It appears that this magically resolves the go.mod file issue. https://stackoverflow.com/questions/67203641/missing-go-sum-entry-for-module-providing-package-package-name Signed-off-by: Fangyi Zhou <[email protected]> * Account for go mod download in go1.17 not updating go.sum (open-telemetry#50) * Update to collector v0.29.0 (open-telemetry#54) * Update replaces.builder.yaml * Update nocore.builder.yaml * Update config.go * Update README.md * Update main.go * Update to collector v0.30.0 (open-telemetry#57) * cmd: fix module flag default value to github.com/open-telemetry (open-telemetry#58) Signed-off-by: Koichi Shiraishi <[email protected]> * Update to collector v0.31.0 (open-telemetry#60) * Update to v0.33.0 (open-telemetry#62) Signed-off-by: Anthony J Mirabella <[email protected]> * Add excludes support to generated go.mod (open-telemetry#63) Signed-off-by: Anthony J Mirabella <[email protected]> Co-authored-by: Juraci Paixão Kröhling <[email protected]> * Small cleanup for the builder files (open-telemetry#64) Signed-off-by: Bogdan Drutu <[email protected]> * Support building with Go 1.17 (open-telemetry#66) * Support building with Go 1.17 Fixes open-telemetry#65 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Update workflows to use Go 1.17 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Add gosec exceptions for exec.Command Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Update to OpenTelemetry core 0.34.0 (open-telemetry#68) Fixes open-telemetry#67 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Upgrade to OpenTelemetry Collector 0.35.0 (open-telemetry#70) Signed-off-by: Fangyi Zhou <[email protected]> * Upgrade to OpenTelemetry Collector 0.36.0 (open-telemetry#76) * Generate custom service code for Windows (open-telemetry#75) * update main to include windows service code * use main version from tag 0.35.0 * update main function * align with upstream v0.36.0 tag * dummy change to trigger build * Revert "dummy change to trigger build" This reverts commit 629d499461da2d2c240bf1e495b5fe0558e3547f. * Remove Core from Module type (open-telemetry#77) Fixes open-telemetry#15 Signed-off-by: yugo-horie <[email protected]> * release 0.37.0 (open-telemetry#78) * release 0.37.0 * update use of NewCommand * Move builder to subdirectory Signed-off-by: Juraci Paixão Kröhling <[email protected]> Co-authored-by: Bogdan Drutu <[email protected]> Co-authored-by: Bogdan Drutu <[email protected]> Co-authored-by: Joe Elliott <[email protected]> Co-authored-by: Eric Yang <[email protected]> Co-authored-by: Brian Gibbins <[email protected]> Co-authored-by: Ashmita <[email protected]> Co-authored-by: Fangyi Zhou <[email protected]> Co-authored-by: Shaun Creary <[email protected]> Co-authored-by: Patryk Małek <[email protected]> Co-authored-by: Serge Catudal <[email protected]> Co-authored-by: Aaron Stone <[email protected]> Co-authored-by: Patryk Małek <[email protected]> Co-authored-by: Aaron Stone <[email protected]> Co-authored-by: Kelvin Lo <[email protected]> Co-authored-by: Himanshu <[email protected]> Co-authored-by: Y.Horie <[email protected]> Co-authored-by: Koichi Shiraishi <[email protected]> Co-authored-by: Anthony Mirabella <[email protected]> Co-authored-by: Cal Loomis <[email protected]> Co-authored-by: alrex <[email protected]>

Add Override Creds Chain

pcwiese force-pushed the pwiese/azuremonitor-trace-exporter branch 9 times, most recently from 417fda9 to 0d63fec Compare November 20, 2019 22:17

pcwiese marked this pull request as ready for review November 20, 2019 22:20

pcwiese requested review from bogdandrutu, flands, owais, pjanotti, rghetia, songy23 and tigrannajaryan as code owners November 20, 2019 22:20

pcwiese changed the title ~~Adding the Azure Monitor trace exporter~~ Adding a trace exporter to Azure Monitor Nov 20, 2019

owais reviewed Nov 20, 2019

View reviewed changes

pcwiese force-pushed the pwiese/azuremonitor-trace-exporter branch 2 times, most recently from 27d3b4c to e8b044e Compare November 21, 2019 06:33

pcwiese closed this Nov 21, 2019

pcwiese reopened this Nov 21, 2019

pcwiese force-pushed the pwiese/azuremonitor-trace-exporter branch 3 times, most recently from 0f0c251 to bed4504 Compare November 21, 2019 22:46

SergeyKanzhelev reviewed Nov 22, 2019

View reviewed changes

exporter/azuremonitorexporter/factory.go Outdated Show resolved Hide resolved

SergeyKanzhelev reviewed Nov 22, 2019

View reviewed changes

exporter/azuremonitorexporter/factory.go Outdated Show resolved Hide resolved

tigrannajaryan reviewed Nov 26, 2019

View reviewed changes

tigrannajaryan mentioned this pull request Nov 26, 2019

Add AWS X-Ray Exporter #41

Merged

tigrannajaryan reviewed Nov 26, 2019

View reviewed changes

exporter/azuremonitorexporter/config_test.go Show resolved Hide resolved

pcwiese force-pushed the pwiese/azuremonitor-trace-exporter branch from e9f1c3b to 6b2082c Compare November 26, 2019 16:52

pcwiese added 11 commits December 2, 2019 08:36

Wiring up the Azure Monitor exporter via local module redirect

41a027f

Adding slightly more test coverage

1c55899

Addressing some PR comments

59b4208

Rename toHex -> idToHex

e3c7ef1

Address more PR comments

d19f65b

Plumb zap.Logger through to the exporter

d1ad3d8

Satisfy impi import validation

1d66d39

Sanitize envelope fix

8544bd9

Numeric attribute values -> measurements collection

3bdc21a

Remove the exporter Debug configuration flag and just use the informa…

3493c62

…tion available in the zap.Logger

pcwiese force-pushed the pwiese/azuremonitor-trace-exporter branch from 3a068cd to 3493c62 Compare December 2, 2019 16:39

pjanotti reviewed Dec 3, 2019

View reviewed changes

exporter/azuremonitorexporter/factory.go Show resolved Hide resolved

go.mod Show resolved Hide resolved

Adding a config validation check

739f9a2

pjanotti approved these changes Dec 3, 2019

View reviewed changes

exporter/azuremonitorexporter/config.go Outdated Show resolved Hide resolved

exporter/azuremonitorexporter/testdata/config.yaml Show resolved Hide resolved

Addressing PR comments

9ca7e19

pjanotti merged commit 27c7118 into open-telemetry:master Dec 3, 2019

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

split core.go (#39)

dc4a44c

hex1848 pushed a commit to hex1848/opentelemetry-collector-contrib that referenced this pull request Jun 2, 2022

Update to 0.26.0 and update BuildInfo (open-telemetry#39)

fe3af5d

sky333999 pushed a commit to sky333999/opentelemetry-collector-contrib that referenced this pull request Jul 3, 2023

Merge pull request open-telemetry#39 from amazon-contributing/override

b6214f6

Add Override Creds Chain

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding a trace exporter to Azure Monitor #39

Adding a trace exporter to Azure Monitor #39

pcwiese commented Nov 19, 2019 •

edited

Loading

owais left a comment

pcwiese commented Nov 21, 2019 •

edited

Loading

pcwiese commented Nov 21, 2019

pcwiese commented Nov 26, 2019

pcwiese commented Nov 26, 2019

pcwiese commented Nov 29, 2019

pjanotti left a comment

pjanotti left a comment

pjanotti commented Dec 3, 2019

Adding a trace exporter to Azure Monitor #39

Adding a trace exporter to Azure Monitor #39

Conversation

pcwiese commented Nov 19, 2019 • edited Loading

owais left a comment

Choose a reason for hiding this comment

pcwiese commented Nov 21, 2019 • edited Loading

pcwiese commented Nov 21, 2019

pcwiese commented Nov 26, 2019

pcwiese commented Nov 26, 2019

pcwiese commented Nov 29, 2019

pjanotti left a comment

Choose a reason for hiding this comment

pjanotti left a comment

Choose a reason for hiding this comment

pjanotti commented Dec 3, 2019

pcwiese commented Nov 19, 2019 •

edited

Loading

pcwiese commented Nov 21, 2019 •

edited

Loading