Skip to content

feat(bottlecap): add Step Functions trace extraction#449

Merged
duncanista merged 13 commits intojordan.gonzalez/bottlecap/universal-instrumentationfrom
jordan.gonzalez/bottlecap/step-functions
Nov 12, 2024
Merged

feat(bottlecap): add Step Functions trace extraction#449
duncanista merged 13 commits intojordan.gonzalez/bottlecap/universal-instrumentationfrom
jordan.gonzalez/bottlecap/step-functions

Conversation

@duncanista
Copy link
Copy Markdown
Contributor

@duncanista duncanista commented Nov 12, 2024

What?

Allows Step Function invokes to Lambda to appear in the same trace

Screenshot 2024-11-12 at 4 24 43 PM Screenshot 2024-11-12 at 4 25 03 PM

How

Generating trace context data through deterministic methods

Notes

  • Step Functions don't need an inferred span, so we remove it in case a generated_span_context exists

@duncanista duncanista requested a review from a team as a code owner November 12, 2024 18:46
DatadogHeaderPropagator, DATADOG_PARENT_ID_KEY, DATADOG_SPAN_ID_KEY,
DATADOG_TRACE_ID_KEY,
},
DatadogCompositePropagator, Propagator,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At point in the invocation is this happening? Is this extracting context from the inbound event? Or is it getting it from the start invocation request headers?

Just asking cuz it doesn't look like we have support for w3c headers. Is that correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure which line you're talking about, you left the comment in the import

Copy link
Copy Markdown
Contributor Author

@duncanista duncanista Nov 12, 2024

Choose a reason for hiding this comment

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

Just asking cuz it doesn't look like we have support for w3c headers. Is that correct?

We do, the composite propagator does that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ahhh, didn't realize this is just the import. Thanks for answering the question!

Copy link
Copy Markdown
Contributor

@avedmala avedmala left a comment

Choose a reason for hiding this comment

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

Live tests and unit tests look good, thanks Jordan!! 🚢

String::new()
}

fn get_carrier(&self) -> HashMap<String, String> {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No carrier, we generate the span context. The decision behind this is that the customer might be using W3C or other style, and we cannot just generate a carrier based on that, its easier to just return a SpanContext

}

#[test]
fn test_generate_parent_id() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These tests are the ones from the Go Agent, so we can see that data is generated as expected.

Copy link
Copy Markdown
Contributor

@agocs agocs left a comment

Choose a reason for hiding this comment

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

LGTM

@duncanista duncanista merged commit 81d2e4f into jordan.gonzalez/bottlecap/universal-instrumentation Nov 12, 2024
@duncanista duncanista deleted the jordan.gonzalez/bottlecap/step-functions branch November 12, 2024 22:37
duncanista added a commit that referenced this pull request Nov 15, 2024
* add step functions events payloads

* make some methods public

* add `StepFunctionEvent`

* adapt `SpanInferrer` for generated `SpanContext`

* adapt `InvocationProcessor` for generated `SpanContext`

* resolve merge conflicts

* resolve clippy issues

* add allow clippy

* do not serialize the `entered_time`

* set `None` for inferred span when `generated_span_context` exists

* tidy code for last trace context update

* fix unit test
duncanista added a commit that referenced this pull request Nov 19, 2024
* add step functions events payloads

* make some methods public

* add `StepFunctionEvent`

* adapt `SpanInferrer` for generated `SpanContext`

* adapt `InvocationProcessor` for generated `SpanContext`

* resolve merge conflicts

* resolve clippy issues

* add allow clippy

* do not serialize the `entered_time`

* set `None` for inferred span when `generated_span_context` exists

* tidy code for last trace context update

* fix unit test
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.

4 participants