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

Adding a trace exporter to Azure Monitor #39

Merged

Conversation

pcwiese
Copy link
Contributor

@pcwiese pcwiese commented Nov 19, 2019

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%

@pcwiese pcwiese force-pushed the pwiese/azuremonitor-trace-exporter branch 9 times, most recently from 417fda9 to 0d63fec Compare November 20, 2019 22:17
@pcwiese pcwiese marked this pull request as ready for review November 20, 2019 22:20
@pcwiese pcwiese changed the title Adding the Azure Monitor trace exporter Adding a trace exporter to Azure Monitor Nov 20, 2019
Copy link
Contributor

@owais owais left a comment

Choose a reason for hiding this comment

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

First pass. Will take another look tomorrow.

exporter/azuremonitorexporter/config.go Outdated Show resolved Hide resolved
exporter/azuremonitorexporter/exporter_common.go Outdated Show resolved Hide resolved
@pcwiese pcwiese force-pushed the pwiese/azuremonitor-trace-exporter branch 2 times, most recently from 27d3b4c to e8b044e Compare November 21, 2019 06:33
@pcwiese
Copy link
Contributor Author

pcwiese commented Nov 21, 2019

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 pcwiese closed this Nov 21, 2019
@pcwiese pcwiese reopened this Nov 21, 2019
@pcwiese pcwiese force-pushed the pwiese/azuremonitor-trace-exporter branch 3 times, most recently from 0f0c251 to bed4504 Compare November 21, 2019 22:46
@pcwiese
Copy link
Contributor Author

pcwiese commented Nov 21, 2019

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 Outdated Show resolved Hide resolved
exporter/azuremonitorexporter/factory.go Outdated Show resolved Hide resolved
exporter/azuremonitorexporter/factory.go Outdated Show resolved Hide resolved
exporter/azuremonitorexporter/factory_test.go Outdated Show resolved Hide resolved
exporter/azuremonitorexporter/factory_test.go Outdated Show resolved Hide resolved
exporter/azuremonitorexporter/traceexporter.go Outdated Show resolved Hide resolved
exporter/azuremonitorexporter/traceexporter.go Outdated Show resolved Hide resolved
exporter/azuremonitorexporter/traceexporter_test.go Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
@pcwiese
Copy link
Contributor Author

pcwiese commented Nov 26, 2019

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

The link is broken.

Fixed.

@pcwiese
Copy link
Contributor Author

pcwiese commented Nov 26, 2019

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 pcwiese force-pushed the pwiese/azuremonitor-trace-exporter branch from e9f1c3b to 6b2082c Compare November 26, 2019 16:52
@pcwiese
Copy link
Contributor Author

pcwiese commented Nov 29, 2019

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%
@pcwiese pcwiese force-pushed the pwiese/azuremonitor-trace-exporter branch from 3a068cd to 3493c62 Compare December 2, 2019 16:39
Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

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 Show resolved Hide resolved
go.mod Show resolved Hide resolved
Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

LGTM.

exporter/azuremonitorexporter/config.go Outdated Show resolved Hide resolved
@pjanotti pjanotti merged commit 27c7118 into open-telemetry:master Dec 3, 2019
@pjanotti
Copy link
Contributor

pjanotti commented Dec 3, 2019

Merged @pcwiese, thanks for the PR.

ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
hex1848 pushed a commit to hex1848/opentelemetry-collector-contrib that referenced this pull request Jun 2, 2022
hex1848 pushed a commit to hex1848/opentelemetry-collector-contrib that referenced this pull request Jun 2, 2022
* 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 <juraci@kroehling.de>

* fixed panics (open-telemetry#6)

Signed-off-by: Joe Elliott <number101010@gmail.com>

* Replace master with main in CI and mergify files (open-telemetry#8)

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>

* Bump to OpenTelemetry Collector 0.20.0 (open-telemetry#10)

Closes open-telemetry#9

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>

* 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 <juraci@kroehling.de>

* 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 <ashmita.bohara152@gmail.com>

* 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 <serge.catudal@gmail.com>

* 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 <juraci@kroehling.de>

Co-authored-by: Juraci Paixão Kröhling <juraci@kroehling.de>

* 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 <juraci.github@kroehling.de>

* 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 <juraci.github@kroehling.de>

* 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 <me@fangyi.io>

* 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 <me@fangyi.io>

* 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 <zchee.io@gmail.com>

* Update to collector v0.31.0 (open-telemetry#60)

* Update to v0.33.0 (open-telemetry#62)

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>

* Add excludes support to generated go.mod (open-telemetry#63)

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>

Co-authored-by: Juraci Paixão Kröhling <juraci@kroehling.de>

* Small cleanup for the builder files (open-telemetry#64)

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

* 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 <juraci@kroehling.de>

* Update workflows to use Go 1.17

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>

* Add gosec exceptions for exec.Command

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>

* Update to OpenTelemetry core 0.34.0 (open-telemetry#68)

Fixes open-telemetry#67

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>

* Upgrade to OpenTelemetry Collector 0.35.0 (open-telemetry#70)

Signed-off-by: Fangyi Zhou <me@fangyi.io>

* 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 <u5.horie@gmail.com>

* 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 <juraci@kroehling.de>

Co-authored-by: Bogdan Drutu <lazy@splunk.com>
Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
Co-authored-by: Joe Elliott <joe.elliott@grafana.com>
Co-authored-by: Eric Yang <jiwen624@gmail.com>
Co-authored-by: Brian Gibbins <eroteme@supernought.co.uk>
Co-authored-by: Ashmita <ashmita.bohara152@gmail.com>
Co-authored-by: Fangyi Zhou <me@fangyi.io>
Co-authored-by: Shaun Creary <65406540+crearys@users.noreply.github.com>
Co-authored-by: Patryk Małek <69143962+pmalek-sumo@users.noreply.github.com>
Co-authored-by: Serge Catudal <serge.catudal@gmail.com>
Co-authored-by: Aaron Stone <aaron@serendipity.cx>
Co-authored-by: Patryk Małek <pmalek@sumologic.com>
Co-authored-by: Aaron Stone <aaron.stone@udacity.com>
Co-authored-by: Kelvin Lo <kello@live.ca>
Co-authored-by: Himanshu <addyjeridiq@gmail.com>
Co-authored-by: Y.Horie <u5.horie@gmail.com>
Co-authored-by: Koichi Shiraishi <zchee.io@gmail.com>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Cal Loomis <68860480+loomis-relativity@users.noreply.github.com>
Co-authored-by: alrex <aboten@lightstep.com>
sky333999 pushed a commit to sky333999/opentelemetry-collector-contrib that referenced this pull request Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants