datadog: event scheduling portion of new tracing library

[dgoffredo]

This is part of the initiative described in #23958.

This revision introduces a new library dependency for Datadog tracing, and adds a component that will be used in the future when more of the new library is integrated into the Datadog tracing extension.

EventScheduler allows the new tracing library to schedule a recurring timer that fires every two seconds; this is when the tracing library will send its batched traces to the Datadog Agent.

[repokitteh-read-only]

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 @moderation

🐱

Caused by: #25170 was opened by dgoffredo.

see: more, trace.

[moderation]

The dependency changes are in multiple PRs - https://github.com/envoyproxy/envoy/pull/25121/files#diff-a2ca502474bb8ec5f092176f7ee6d2f11fc395ebb00e1717d6fbad9b16ee8fd8

Is this intentional?

[dgoffredo]

Yes, it's intentional. In retrospect, it might have been better to first add only the dependency changes, wait for those changes to be merged into main, and then propose the lowest level of new code.

I wanted instead to propose the first round of pull requests ASAP, and so duplicated the shared added dependency in all of those lowest level PRs.

I could create a dedicated dependency PR and rebase the others onto it, if you like.

[wbpcode]

ping myself, sorry for the delay. 😞

[wbpcode]

Thanks for you contribution and some comments are added. And please merge the main to resolve the conflict.

[dgoffredo]

Thanks for the review. I'll push a commit and merge from main within the next couple of days.

[wbpcode]

/wait ci and conflict.

[dgoffredo]

Code coverage for source/extensions/tracers/datadog is lower than limit of 96.6 (96.5)

Hmmm. Looks like calling config_json in a test might help: https://storage.googleapis.com/envoy-pr/0f305a0/coverage/source/extensions/tracers/datadog/event_scheduler.cc.gcov.html

Also interesting in the logger, which was recently merged, I'd except these kinds of tests: https://github.com/envoyproxy/envoy/pull/25025/files#diff-ab9f820e99ec92b52dd7334d825976ab5b5e2240dc9693e58fd4c99fab4c7dddR96-R103
to cover these kinds of branches: https://storage.googleapis.com/envoy-pr/9763eb9/coverage/source/extensions/tracers/datadog/logger.cc.gcov.html
but they don't.

I'll have to look closer.

[edit]: Ah, the logger and its sink(s) have separate levels. It's the logger level that matters most.

[wbpcode]

LGTM. Thanks for you great contribution.

cc @jmarantz for final review or merge.

[dgoffredo]

Ping @jmarantz

[jmarantz]

Sorry for the delay this fell off my radar.

I found a few minor suggestions, but I'm not going to be available for the next few days. Whatever you decide is fine.

@wbpcode feel free to make the final call and merge for this.

[jmarantz]

nit: don't use an underscore suffix for a local. Comment on memory management: who deletes timer?

[dgoffredo]

This pattern is copied from here:

envoy/test/extensions/tracers/datadog/datadog_tracer_impl_test.cc

Line 54 in 40d15a9

timer_ = new NiceMock<Event::MockTimer>(&tls_.dispatcher_);

Which was copied from here:

envoy/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc

Line 55 in 40d15a9

timer_ = new NiceMock<Event::MockTimer>(&tls_.dispatcher_);

It occurs to me now that the raw pointer is probably a substitute for "optional" in the original code.

who deletes timer?

Nobody, it's leaked, as in the copied-from code.

I'll see if it works the same with just a testing::NiceMock<Event::MockTimer> on the stack (no pointer). That'd be better.

[dgoffredo]

Ah, it's not for lack of optional. This is indeed the intended usage of MockTimer:

envoy/test/mocks/event/mocks.h

Lines 183 to 186 in eee9b7c

	// Ownership of each MockTimer instance is transferred to the (caller of) dispatcher's
	// createTimer_(), so to avoid destructing it twice, the MockTimer must have been dynamically
	// allocated and must not be deleted by it's creator.
	MockTimer(MockDispatcher* dispatcher);

[jmarantz]

dito

[dgoffredo]

Thanks!

Another week has gone by and I still haven't produced the final pair of pull requests. Those are coming soon...