Skip to content

chore(tracing): update dd-trace-cpp to 0.1.11#29354

Closed
sebbrandt87 wants to merge 1 commit into
envoyproxy:mainfrom
Tchibo:chore/update-datadog-dd-trace-cpp
Closed

chore(tracing): update dd-trace-cpp to 0.1.11#29354
sebbrandt87 wants to merge 1 commit into
envoyproxy:mainfrom
Tchibo:chore/update-datadog-dd-trace-cpp

Conversation

@sebbrandt87
Copy link
Copy Markdown

@sebbrandt87 sebbrandt87 commented Aug 31, 2023

Fix #30552

Commit Message:
chore(tracing): update dd-trace-cpp to 0.1.11

Additional Description:

  • include new propagation style b3multi compatibility

see https://github.com/DataDog/dd-trace-cpp/blob/v0.1.11/src/datadog/propagation_style.cpp#L39-L49

Signed-off-by: Brandt, Sebastian (ITO-PC) sebastian.brandt@tchibo.de

Risk Level: Low
Testing: through implemented testing already present
Docs Changes: N/A
Release Notes: Yes
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link
Copy Markdown

Hi @sebbrandt87, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #29354 was opened by sebbrandt87.

see: more, trace.

@repokitteh-read-only repokitteh-read-only Bot added v2-freeze api deps Approval required for changes to Envoy's external dependencies labels Aug 31, 2023
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto).
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @phlax

🐱

Caused by: #29354 was opened by sebbrandt87.

see: more, trace.

@phlax
Copy link
Copy Markdown
Member

phlax commented Aug 31, 2023

@sebbrandt87 there are a large number of changes here that seem unrelated to the PR

my guess is that you ran some of the tooling outside of our build container and with incorrect versions

@sebbrandt87
Copy link
Copy Markdown
Author

@sebbrandt87 there are a large number of changes here that seem unrelated to the PR

my guess is that you ran some of the tooling outside of our build container and with incorrect versions

indeed, let me recheck and recommit, sorry for the inconvenience

@sebbrandt87 sebbrandt87 force-pushed the chore/update-datadog-dd-trace-cpp branch from 4b4a8b4 to d09f065 Compare August 31, 2023 12:12
phlax
phlax previously approved these changes Aug 31, 2023
Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

lgtm, pending CI, thanks @sebbrandt87

@repokitteh-read-only repokitteh-read-only Bot removed the deps Approval required for changes to Envoy's external dependencies label Aug 31, 2023
Comment thread changelogs/current.yaml Outdated
@repokitteh-read-only repokitteh-read-only Bot added the deps Approval required for changes to Envoy's external dependencies label Aug 31, 2023
phlax
phlax previously approved these changes Aug 31, 2023
@repokitteh-read-only repokitteh-read-only Bot removed the deps Approval required for changes to Envoy's external dependencies label Aug 31, 2023
@phlax
Copy link
Copy Markdown
Member

phlax commented Aug 31, 2023

In file included from bazel-out/aarch64-opt/bin/external/com_github_datadog_dd_trace_cpp/_virtual_includes/dd_trace_cpp/datadog/tracer.h:15:
bazel-out/aarch64-opt/bin/external/com_github_datadog_dd_trace_cpp/_virtual_includes/dd_trace_cpp/datadog/expected.h:108:9: error: object of type 'std::variant<Span, Error>' cannot be assigned because its copy assignment operator is implicitly deleted
  data_ = std::forward<Other>(other);
        ^
source/extensions/tracers/datadog/tracer.cc:109:16: note: in instantiation of function template specialization 'datadog::tracing::Expected<datadog::tracing::Span>::operator=<datadog::tracing::Span>' requested here
    maybe_span = tracer.create_span(span_config);
               ^
/opt/llvm/bin/../include/c++/v1/variant:1362:12: note: explicitly defaulted function was implicitly deleted here
  variant& operator=(const variant&) = default;
           ^
/opt/llvm/bin/../include/c++/v1/variant:1262:7: note: copy assignment operator of 'variant<datadog::tracing::Span, datadog::tracing::Error>' is implicitly deleted because base class '__sfinae_assign_base<__all<(is_copy_constructible_v<Span> && is_copy_assignable_v<Span>), (is_copy_constructible_v<Error> && is_copy_assignable_v<Error>)>::value, __all<(is_move_constructible_v<Span> && is_move_assignable_v<Span>), (is_move_constructible_v<Error> && is_move_assignable_v<Error>)>::value>' has a deleted copy assignment operator
      private __sfinae_assign_base<
      ^
/opt/llvm/bin/../include/c++/v1/__tuple:528:25: note: 'operator=' has been explicitly marked deleted here
  __sfinae_assign_base& operator=(__sfinae_assign_base const&) = delete;
                        ^

https://dev.azure.com/cncf/envoy/_build/results?buildId=147800&view=logs&j=767be981-567e-57d8-68c3-2140ede0a0bd&t=8a34f9ed-4946-5fcf-7cd6-99c93070884d&l=632

@phlax
Copy link
Copy Markdown
Member

phlax commented Sep 5, 2023

@sebbrandt87 i think the above fail looks real

Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

Thanks for doing the cross-repo fixes.
Left a small comment (also need to fix the conflict), but otherwise LGTM.

Comment thread changelogs/current.yaml Outdated
@wbpcode
Copy link
Copy Markdown
Member

wbpcode commented Oct 20, 2023

/wait

Waiting for main merging and comments to be resolved.

@sebbrandt87 sebbrandt87 force-pushed the chore/update-datadog-dd-trace-cpp branch from b8fca50 to 7ad4c16 Compare October 20, 2023 10:16
adisuissa
adisuissa previously approved these changes Oct 20, 2023
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@adisuissa
Copy link
Copy Markdown
Contributor

Please fix CI error

@adisuissa
Copy link
Copy Markdown
Contributor

CI error seems relevant, can you PTAL?
/wait

@sebbrandt87
Copy link
Copy Markdown
Author

I try my best with spare time I will find. Any advice or helping hand in solving much appreciated. 🤝

@adisuissa
Copy link
Copy Markdown
Contributor

I try my best with spare time I will find. Any advice or helping hand in solving much appreciated. 🤝

I don't have cycles to address this. The cause for the issue is described in a prior comment.
This needs either a patch when importing the dependency, or updating the external dependency and importing the newer version (preferred).

/wait

@sebbrandt87
Copy link
Copy Markdown
Author

sebbrandt87 commented Oct 27, 2023

I try my best with spare time I will find. Any advice or helping hand in solving much appreciated. 🤝

I don't have cycles to address this. The cause for the issue is described in a prior comment. This needs either a patch when importing the dependency, or updating the external dependency and importing the newer version (preferred).

/wait

Okay.
DataDog/dd-trace-cpp@691b190 since then it seems to be forbidden to assign to the Span directly.

@phlax
Copy link
Copy Markdown
Member

phlax commented Oct 27, 2023

0.1.11 is available #30552

@sebbrandt87 sebbrandt87 force-pushed the chore/update-datadog-dd-trace-cpp branch from 07120c9 to b489ff2 Compare October 30, 2023 13:52
@sebbrandt87 sebbrandt87 changed the title chore(tracing): update dd-trace-cpp to 0.1.10 chore(tracing): update dd-trace-cpp to 0.1.11 Oct 30, 2023
- include new propagation style b3multi compatibility

see https://github.com/DataDog/dd-trace-cpp/blob/v0.1.11/src/datadog/propagation_style.cpp#L39-L49

Co-authored-by: phlax <phlax@users.noreply.github.com>

Signed-off-by: Brandt, Sebastian (ITO-PC) <sebastian.brandt@tchibo.de>
@sebbrandt87 sebbrandt87 force-pushed the chore/update-datadog-dd-trace-cpp branch from b489ff2 to b520b0a Compare October 30, 2023 13:58
@adisuissa
Copy link
Copy Markdown
Contributor

Still same error as before, waiting for a solution to the underlying issue.
/wait

RyanTheOptimist pushed a commit that referenced this pull request Jan 5, 2024
dd-trace-cpp is Datadog's core C++ tracing library, and is used by Envoy to provide tracing via Datadog.

Envoy is currently consuming an older version of dd-trace-cpp. Subsequent releases of dd-trace-cpp were not compatible with the Datadog tracing extension here.

These changes make the Datadog tracing extension compatible with the latest release of dd-trace-cpp, v0.1.12.

The changes are mostly in unit tests. Newer versions of dd-trace-cpp send more HTTP requests to the Datadog Agent, and so tests that assumed the number of requests were broken. There are also some changes involving how timeouts are handled by dd-trace-cpp.

These changes address #30957, #29354, and #31360.

Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
@phlax
Copy link
Copy Markdown
Member

phlax commented Jan 9, 2024

@sebbrandt87 i think this has been superseded we are now up-to-date for this dep - so im going to close - thanks for working on it

@phlax phlax closed this Jan 9, 2024
Smeb pushed a commit to Smeb/envoy that referenced this pull request Jan 24, 2024
dd-trace-cpp is Datadog's core C++ tracing library, and is used by Envoy to provide tracing via Datadog.

Envoy is currently consuming an older version of dd-trace-cpp. Subsequent releases of dd-trace-cpp were not compatible with the Datadog tracing extension here.

These changes make the Datadog tracing extension compatible with the latest release of dd-trace-cpp, v0.1.12.

The changes are mostly in unit tests. Newer versions of dd-trace-cpp send more HTTP requests to the Datadog Agent, and so tests that assumed the number of requests were broken. There are also some changes involving how timeouts are handled by dd-trace-cpp.

These changes address envoyproxy#30957, envoyproxy#29354, and envoyproxy#31360.

Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Smeb pushed a commit to Smeb/envoy that referenced this pull request Jan 24, 2024
dd-trace-cpp is Datadog's core C++ tracing library, and is used by Envoy to provide tracing via Datadog.

Envoy is currently consuming an older version of dd-trace-cpp. Subsequent releases of dd-trace-cpp were not compatible with the Datadog tracing extension here.

These changes make the Datadog tracing extension compatible with the latest release of dd-trace-cpp, v0.1.12.

The changes are mostly in unit tests. Newer versions of dd-trace-cpp send more HTTP requests to the Datadog Agent, and so tests that assumed the number of requests were broken. There are also some changes involving how timeouts are handled by dd-trace-cpp.

These changes address envoyproxy#30957, envoyproxy#29354, and envoyproxy#31360.

Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deps Approval required for changes to Envoy's external dependencies no stalebot Disables stalebot from closing an issue waiting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Newer release available com_github_datadog_dd_trace_cpp: v0.1.11 (current: v0.1.8)

5 participants