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

Remove translation to HTTP status from OC status. #2978

Merged

Conversation

anuraaga
Copy link
Contributor

@anuraaga anuraaga commented Apr 2, 2021

Description: Remove compatibility adapter for OC for mapping HTTP status.

Link to tracking Issue: Fixes #2940

Testing: Unit tests

This was here to give time for OC users to migrate instrumentation - if @kbrockhoff is ok with it, we can delete it now.

@anuraaga anuraaga requested a review from kbrockhoff as a code owner April 2, 2021 02:33
@anuraaga anuraaga requested review from a team and bogdandrutu April 2, 2021 02:33
@codecov
Copy link

codecov bot commented Apr 2, 2021

Codecov Report

Merging #2978 (80486ae) into main (101b75e) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2978      +/-   ##
==========================================
- Coverage   91.53%   91.52%   -0.01%     
==========================================
  Files         464      464              
  Lines       22799    22797       -2     
==========================================
- Hits        20868    20864       -4     
- Misses       1438     1439       +1     
- Partials      493      494       +1     
Flag Coverage Δ
integration 68.96% <ø> (ø)
unit 90.48% <ø> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
exporter/awsxrayexporter/translator/http.go 92.14% <ø> (-0.12%) ⬇️
receiver/k8sclusterreceiver/watcher.go 95.29% <0.00%> (-2.36%) ⬇️

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 101b75e...80486ae. Read the comment docs.

@bogdandrutu
Copy link
Member

@anuraaga this is not going to help OC users, since data are translated OC -> Otel -> Xray and it is important that OC -> Otel does the right thing, otherwise you will not have the right values anyway.

@bogdandrutu bogdandrutu merged commit e171581 into open-telemetry:main Apr 2, 2021
pmatyjasek-sumo pushed a commit to pmatyjasek-sumo/opentelemetry-collector-contrib that referenced this pull request Apr 28, 2021
alexperez52 referenced this pull request in open-o11y/opentelemetry-collector-contrib Aug 18, 2021
* Update mdatagen to create factories of init instead of new

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

* Revert New deletion, too many tests use that

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
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.

awsxrayexporter: wrong usage of tracetranslator.HTTPStatusCodeFromOCStatus
3 participants