datadog: span 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.

This revision also contains all of the changes from #25003, on which it depends. When that PR is merged, the diff of this PR will shrink.

[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: #25121 was opened by dgoffredo.

see: more, trace.

[RyanTheOptimist]

/assign @jmarantz

[moderation]

/lgtm deps

[dgoffredo]

ERROR: From ./test/extensions/tracers/datadog/span_test.cc
ERROR: ./test/extensions/tracers/datadog/span_test.cc:114: Don't use mktime; use absl::Time equivalent instead
ERROR: check format failed. diff has been applied'

I guess I'll use absl::FromCivil and absl::ToChronoTime instead.

[jmarantz]

This looks great; really nicely tested. Just a few nits.

I think @moderation should look at this also for span semantics.

[jmarantz]

should we use an ENVOY_BUG here (logs a message in production build, asserts in debug/default builds)?

[dgoffredo]

That might be worth doing.

I'd reserve assert and friends for cases where the detected behavior is in violation of the component's contract. Because this Span has the escape hatch of going into "null-mode," it's not necessary. By that reasoning, perhaps the code comment is not necessary either.

You mentioned the idea of @moderation looking over the behavior of this Span. Maybe they can decide whether "spawn-after-finish" ought to be considered forbidden in these tracing implementations.

[wbpcode]

Please merge main and resolve the conflict first. Thanks.

[wbpcode]

LGTM overall. It's great. Only some minor comments.

[jmarantz]

this all looks good from my perspective modulo @wbpcode comments.

I think it might need a main merge to kick off CI also.

[dgoffredo]

#24826 ☹️

[dgoffredo]

test/extensions/tracers/datadog/span_test.cc:292:47: error: expression with side effects will be evaluated despite being used as an operand to 'typeid' [-Werror,-Wpotentially-evaluated-expression]
  EXPECT_EQ(typeid(Tracing::NullSpan), typeid(*child));
                                              ^
1 error generated.

That's a new one.

I suppose instead of this:

  const Tracing::SpanPtr child =
      span.spawnChild(Tracing::MockConfig{}, "child", time_.timeSystem().systemTime());
  EXPECT_NE(nullptr, child);
  EXPECT_EQ(typeid(Tracing::NullSpan), typeid(*child));

it wants this:

  const Tracing::SpanPtr child =
      span.spawnChild(Tracing::MockConfig{}, "child", time_.timeSystem().systemTime());
  EXPECT_NE(nullptr, child);
  Tracing::Span& child_span = *child;
  EXPECT_EQ(typeid(Tracing::NullSpan), typeid(child_span));

I'll make that change and see if it helps.

[jmarantz]

@moderation I've lgtm'd can you look this over and merge if you are good with it?

[moderation]

@jmarantz I don't have merge permissions but happy to see this move ahead as it will allow us to remove some old dependencies. Maybe @phlax can merge?

[zirain]

@dgoffredo Is this right?