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

elasticexporter: don't return error on invalid tags #285

Merged
merged 1 commit into from
Jun 6, 2020

Conversation

axw
Copy link
Contributor

@axw axw commented Jun 5, 2020

Description:

If an http.url attribute is provided, and does not conform to the
opentelemetry conventions, do not return an error if URL parsing fails.
Instead, just record the value as-is in a label.

For example, the OpenTracing instrumentation for Go's net/http client sends host:port in some cases: https://github.com/opentracing-contrib/go-stdlib/blob/cf7a6c988dc994e945d2715565026f3cc8718689/nethttp/client.go#L255

Testing:

Tested manually with Jaeger HotR.O.D. -> OpenTelemetry Collector -> Elastic APM
(This is how we found the issue in the first place.)

Added unit tests.

@axw axw requested a review from a team June 5, 2020 03:56
If `http.url` is provided, and does not conform to the
opentelemetry conventions, do not return an error if
URL parsing fails. Instead, just record the value as-is
in a label.

For example, the OpenTracing instrumentation for Go's
net/http client sends host:port in some cases:
  https://github.com/opentracing-contrib/go-stdlib/blob/cf7a6c988dc994e945d2715565026f3cc8718689/nethttp/client.go#L255
@axw axw force-pushed the elastic-parse-invalid-urls branch from 3b7c996 to f1a3428 Compare June 5, 2020 05:08
@codecov
Copy link

codecov bot commented Jun 5, 2020

Codecov Report

Merging #285 into master will increase coverage by 0.20%.
The diff coverage is 80.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #285      +/-   ##
==========================================
+ Coverage   79.24%   79.44%   +0.20%     
==========================================
  Files         163      163              
  Lines        8354     8348       -6     
==========================================
+ Hits         6620     6632      +12     
+ Misses       1369     1357      -12     
+ Partials      365      359       -6     
Impacted Files Coverage Δ
...asticexporter/internal/translator/elastic/utils.go 86.95% <ø> (+17.99%) ⬆️
...sticexporter/internal/translator/elastic/traces.go 90.81% <80.00%> (+4.08%) ⬆️

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 a1dba16...f1a3428. Read the comment docs.

@bogdandrutu bogdandrutu merged commit e3c5c8d into open-telemetry:master Jun 6, 2020
@axw axw deleted the elastic-parse-invalid-urls branch June 8, 2020 02:30
wyTrivail referenced this pull request in mxiamxia/opentelemetry-collector-contrib Jul 13, 2020
If `http.url` is provided, and does not conform to the
opentelemetry conventions, do not return an error if
URL parsing fails. Instead, just record the value as-is
in a label.

For example, the OpenTracing instrumentation for Go's
net/http client sends host:port in some cases:
  https://github.com/opentracing-contrib/go-stdlib/blob/cf7a6c988dc994e945d2715565026f3cc8718689/nethttp/client.go#L255
mxiamxia referenced this pull request in mxiamxia/opentelemetry-collector-contrib Jul 22, 2020
* Rename fanoutprocessor to FanOutConnector

This implements open-telemetry/opentelemetry-collector#283

* PR fixes
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* bump dependent module versions.

* update example hashes too.
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.

4 participants