Skip to content

upgrade thrift to v0.14.1 in jaeger exporter#1712

Merged
MrAlias merged 5 commits intoopen-telemetry:mainfrom
kjschnei001:jaeger-thrift-0.14.1
Mar 22, 2021
Merged

upgrade thrift to v0.14.1 in jaeger exporter#1712
MrAlias merged 5 commits intoopen-telemetry:mainfrom
kjschnei001:jaeger-thrift-0.14.1

Conversation

@kjschnei001
Copy link
Copy Markdown
Contributor

With CVE-2020-13949 in mind, this PR upgrades the version of the vendored thrift library in the jaeger exporter from v0.13.0 to v0.14.1. It was not super clear to me why the thrift dependencies were packaged in this way, but I did my best to adhere to the established patterns as best I could, with respect to how the libraries were packaged into the exporter.

The thrift code was incorporated from a vendored version of the v0.14.1 library and I used thrift version 0.14.1 to auto-generate the jaeger-specific libraries from the jaeger-idl, modifying it only to establish new dependency paths within the opentracing project itself.

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Mar 18, 2021

CLA Signed

The committers are authorized under a signed CLA.

@Aneurysm9
Copy link
Copy Markdown
Member

This shouldn't have dependencies under exporters/trace/jaeger/vendor/. The thrift dependency was manually 'vendored' under exporters/trace/jaeger/internal/third_party to avoid issues arising from attempting to use vendor/ in a library package. See #1548 and #1551 for some context on why it was done.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 18, 2021

Codecov Report

Merging #1712 (2292e58) into main (5a6a854) will decrease coverage by 0.0%.
The diff coverage is 50.0%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #1712     +/-   ##
=======================================
- Coverage   77.8%   77.8%   -0.1%     
=======================================
  Files        131     131             
  Lines       6985    6984      -1     
=======================================
- Hits        5439    5437      -2     
- Misses      1296    1297      +1     
  Partials     250     250             
Impacted Files Coverage Δ
exporters/trace/jaeger/uploader.go 56.4% <0.0%> (ø)
exporters/trace/jaeger/agent.go 58.9% <66.6%> (+1.4%) ⬆️
exporters/otlp/otlpgrpc/connection.go 86.9% <0.0%> (-1.9%) ⬇️

Copy link
Copy Markdown
Member

@XSAM XSAM left a comment

Choose a reason for hiding this comment

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

👍

@MrAlias MrAlias merged commit a9b2f85 into open-telemetry:main Mar 22, 2021
@MrAlias MrAlias mentioned this pull request Apr 23, 2021
@pellared pellared added this to the untracked milestone Nov 8, 2024
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