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

Updated AWS X-Ray receiver Config to use udp #497

Merged

Conversation

hencrice
Copy link
Contributor

@hencrice hencrice commented Jul 23, 2020

Description:
The receiver exposes 2 endpoints:

  1. An UDP endpoint for ingesting incoming spans
  2. A TCP endpoint that serves as a local proxy between AWS and X-Ray SDK

This PR updates the Config to correctly store both.

Link to tracking Issue:
Closes #496

Testing:
Unit tests updated

Documentation:
README.md updated

The receiver exposes 2 endpoints:
1. An UDP endpoint for ingesting incoming spans
2. A TCP endpoint that serves as a local proxy between AWS and X-Ray SDK

This PR updates the `Config` to correctly store both.

Closes #496
@hencrice hencrice requested a review from a team as a code owner July 23, 2020 18:08
@project-bot project-bot bot added this to In progress in Collector Jul 23, 2020
@codecov
Copy link

codecov bot commented Jul 23, 2020

Codecov Report

Merging #497 into master will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #497      +/-   ##
==========================================
- Coverage   85.93%   85.92%   -0.02%     
==========================================
  Files         187      187              
  Lines       10055    10058       +3     
==========================================
+ Hits         8641     8642       +1     
- Misses       1094     1095       +1     
- Partials      320      321       +1     
Flag Coverage Δ
#integration 71.09% <ø> (ø)
#unit 85.74% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
receiver/awsxrayreceiver/factory.go 100.00% <100.00%> (ø)
receiver/carbonreceiver/transport/tcp_server.go 65.71% <0.00%> (-1.91%) ⬇️

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 a2ff1aa...93f5cb2. Read the comment docs.

@hencrice
Copy link
Contributor Author

@bogdandrutu @james-bebbington Could you guys review this fix? Thanks!

Collector automation moved this from In progress to Reviewer approved Jul 23, 2020
@bogdandrutu bogdandrutu merged commit 03c7469 into open-telemetry:master Jul 23, 2020
Collector automation moved this from Reviewer approved to Done Jul 23, 2020
@hencrice hencrice deleted the x-ray-receiver/endpointFixes branch July 28, 2020 23:59
dyladan referenced this pull request in dynatrace-oss-contrib/opentelemetry-collector-contrib Jan 29, 2021
Workers were unnecessary complication and did not provide any measurable
performance gains.

In addition they were decoupling receiver from subsequent processors which
makes difficult upcoming implementation of backpressure in the pipeline
(from processors to receivers).

Test results that shows that worker give no performance benefits (any
differences are within the measurement error margin):

Without workers (this commit):

Test                                    |Result|Duration|CPU Avg%|CPU Max%|RAM Avg MiB|RAM Max MiB|Sent Items|Received Items|
----------------------------------------|------|-------:|-------:|-------:|----------:|----------:|---------:|-------------:|
Trace10kSPS/OpenCensus                  |PASS  |     16s|    22.7|    25.9|         37|         45|    149880|        149880|

With workers (latest `master` branch):

Test                                    |Result|Duration|CPU Avg%|CPU Max%|RAM Avg MiB|RAM Max MiB|Sent Items|Received Items|
----------------------------------------|------|-------:|-------:|-------:|----------:|----------:|---------:|-------------:|
Trace10kSPS/OpenCensus                  |PASS  |     16s|    23.3|    24.9|         37|         46|    149920|        149920|
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* opentelemetry collector exporter

- missing load test
- missing resources

* fix review comments.

* add test for each SpanKind and Attribute Type.

* rename otelcol to otlp

* move exporter/trace/otlp to exporters/otlp

* more review comments.

* add alignment test.

* pass context to uploadSpans
codeboten pushed a commit that referenced this pull request Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Collector
  
Done
Development

Successfully merging this pull request may close these issues.

AWS X-Ray receiver uses both UDP and TCP endpoint
2 participants