-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
datadog: Config (final) portion of new tracing library #26284
datadog: Config (final) portion of new tracing library #26284
Conversation
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
….goffredo/dd-trace-cpp-3/9-tracer_stats' of github.com:DataDog/envoy into david.goffredo/dd-trace-cpp-7/9-agent_http_client Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
This reverts commit 570d9f0. Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
I'm currently working on modifications to the
The relevant feature is documented here: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/trace/v3/dynamic_ot.proto It allows a user to bring their own build of an OpenTracing-compatible tracer and 🤷 If you're willing to remove that feature, then the |
In fact, I have no strong point to this. Because I don't know if there are actual users of If the community think we should keep these extensions, I can add some tests to improve the coverage in the next week and unblock this PR (I will have some free time at that time.) |
If you can resolve it directly it would great. |
…ersReader::ForeachKey Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
I pushed a commit that covers the following code: envoy/source/extensions/tracers/common/ot/opentracing_driver_impl.cc Lines 68 to 75 in 4c12d8e
by adding some new boilerplate to the The difficulty is that the existing unit test uses Instead, I reproduce a skeleton of an |
sweet! No changes to core code and no changes to coverage so I'll recuse myself and let josh merge. |
looks like clang-tidy failed though so you may want to clean that up so it' good to merge :-) |
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
@alyssawilk we are go for launch, stand by for ignition sequence |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alyssawilk has deferred to me. LGTM!
I downloaded the zipped source code from the v1.26.1 release, and it does not include the changes in this PR, thus v1.26.1 contains the old extension, not the new one. Might a subsequent release in the v1.26.x series contain these changes, or will we have to wait for v1.27.x? I'd prefer to have these changes available as soon as possible. The changes were initially proposed in November of 2022. edit: It looks like we missed the release cutoff by six hours! |
its possible we could backport the changes to 1.26.x, but i think that we should not as that is usually reserved for fixes the 1.27 release is due for 2023/07/14 |
I agree that a backport would not be appropriate here. Ok, July it is. Thanks @phlax |
) This is the final part of the work proposed in envoyproxy#23958. This PR additionally contains all of the changes from envoyproxy#26148. This diff will shrink when that PR is merged. This revision alters the DatadogTracerFactory in source/extensions/tracers/datadog/config.h to produce instances of the new dd-trace-cpp based implementation of the Datadog tracer, instead of the old dd-opentracing-cpp based one. This revision actually alters the implementation of the Datadog tracer. Previous pull requests have only added code for use here. This revision also moves some unit tests around: Two tests that were part of datadog_tracer_impl_test.cc have been moved into agent_http_client_test.cc, because they're about HTTP requests. One test that was part of datadog_tracer_impl_test.cc has been removed because its behavior is already covered by tests added in other PRs in this series. The remaining tests in datadog_tracer_impl_test_.cc were moved into config_test.cc, since they all now have to do with the configuration of the tracer and the resulting behavior. Finally, all references to dd-opentracing-cpp and msgpack have been removed from the repo, and the documentation (READMEs) and demo/ directory from envoyproxy#23958 have been added. Signed-off-by: David Goffredo <david.goffredo@datadoghq.com> Signed-off-by: Ryan Eskin <ryan.eskin89@protonmail.com>
This is the final part of the work proposed in #23958.
This PR additionally contains all of the changes from #26148. This diff will shrink when that PR is merged.This revision alters the
DatadogTracerFactory
insource/extensions/tracers/datadog/config.h
to produce instances of the new dd-trace-cpp based implementation of the Datadog tracer, instead of the old dd-opentracing-cpp based one.This revision actually alters the implementation of the Datadog tracer. Previous pull requests have only added code for use here.
This revision also moves some unit tests around:
datadog_tracer_impl_test.cc
have been moved intoagent_http_client_test.cc
, because they're about HTTP requests.datadog_tracer_impl_test.cc
has been removed because its behavior is already covered by tests added in other PRs in this series.datadog_tracer_impl_test_.cc
were moved intoconfig_test.cc
, since they all now have to do with the configuration of the tracer and the resulting behavior.Finally, all references to dd-opentracing-cpp and msgpack have been removed from the repo, and the documentation (READMEs) and
demo/
directory from #23958 have been added.