-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
DOCS-9587: restructure step functions install page #26825
base: master
Are you sure you want to change the base?
Conversation
Preview links (active after the
|
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.
Thanks for improving the doc!
|
||
For more information about the `datadog-ci stepfunctions` command, see the [Datadog CLI documentation][4]. | ||
|
||
1. Set up tags. Open your AWS console and go to your Step Functions state machine. Open the *Tags* section and add the following key-value pairs: |
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.
I don't think this manual step is needed. datadog-ci
command will add these tags. Is this section copied from somewhere?
| `dd_version` | `dd_sls_plugin`| | ||
|
||
1. To enable tracing, you have two options: | ||
- **Per Step Function**: Add the `DD_TRACE_ENABLED` tag to each Step Function and set the value to `true`. |
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.
We can add something like: The datadog-ci stepfunctions instrument
command already does this, so no extra work is needed.
|
||
For additional settings, see [Datadog Serverless Framework Plugin - Configuration parameters][5]. | ||
|
||
1. Set up tags. Open your AWS console and go to your Step Functions state machine. Open the *Tags* section and add the following key-value pairs: |
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.
This is not needed. Serverless Plugin will set these tags.
| `dd_version` | `dd_sls_ci`| | ||
|
||
1. To enable tracing, you have two options: | ||
- **Per Step Function**: Add the `DD_TRACE_ENABLED` tag to each Step Function and set the value to `true`. |
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.
We only need to add enableStepFunctionsTracing: true
, as shown in the example code above.
``` | ||
/aws/vendedlogs/states/<CDK_PATH>-Logs | ||
``` | ||
|
||
**Optionally**, you can add your environment to this log group name: `/aws/vendedlogs/states/<CDK_PATH>-Logs-<ENV>` |
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.
With CDK, the user won't need to create the log group on their own.
Also, the instructions for CDK are incomplete. Is there a plan to write this part in a separate PR?
|
||
[1]: https://github.com/DataDog/datadog-cdk-constructs/tree/main/examples/step-functions-python-stack | ||
{{% /tab %}} | ||
{{% tab "Go" %}} |
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.
Just a question, but do we only have the official CDK constructs available in Go, TS and Python? And no support for .NET/Java right?
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.
That's right. I added Go in August 2024. Let me know if there's a need to support .NET or Java.
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.
.NET & Java definitely aren't as widely used, I'm just thinking for completeness that's all. So we can say we support all runtimes.
Is there not a way with the CDK to write it once in Javascript and then auto-gen the other runtimes though?
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.
This is a CDK construct for defining .NET Lambda functions. The construct is defined once in Typescript and then uses Projen to generate all the other packages. Could we not do the same for our constructs?
https://github.com/cdklabs/awscdk-lambda-dotnet/blob/main/.github/workflows/release.yml
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.
Yes, I think it's 1–2 week one-time work to support a new language.
From what I understand, we often use a "lazy" approach that we don't build something until some customer asks for it. Have you heard of any customer ask, or could you align with Sumedha on this?
I like the write-up, some more general added separately to make the conversation easier :) Do we want to add an example in Terraform as well? Maybe not so useful for serverless app developers, but I've seen a lot of usage of StepFunctions for infra/platform level tasks (account provisioning etc). Ancedotally, folks doing those kind of tasks may want to use Terraform. I've got TF examples in my serverless sample app and you'd only need one to cover all runtimes |
I think we should add some content on how a .NET or Java developer would do this. Even if we don't have the Datadog SFN CDK construct it would be good to get specific to show a .NET/Java developer how they would do it in the CDK. Again I have examples in my sample app repo if you want me to share. |
I don't see anything about AWS SAM, but it might be that SAM is already in the docs and not relevant for this PR. |
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.
Otherwise LGTM! I like this new structure.
jsii.String("DatadogSfn"), | ||
&ddcdkconstruct.DatadogStepFunctionsProps{ | ||
Env: jsii.String("<ENV>"), // e.g. "dev" | ||
Service: jsii.String("<SERVICE>), // e.g. "my-cdk-service" |
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.
"<SERVICE>"
misses a quote. Could you correct it here? Sorry I missed it in the original README.
|
||
Take note of your Forwarder's ARN. | ||
|
||
1. Add the following to your `serverless.yml`: |
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.
I think this should be something like
Run the following command:
or just
Run:
Only the Serverless Plugin approach has serverless.yml
.
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.
whoops, copy/paste error. correcting
I don't know a good way for a .NET/Java developer. I think we can add something like:
|
What does this PR do? What is the motivation?
work in progress
Merge instructions
Merge readiness:
Merge queue is enabled in this repo. To have it automatically merged after it receives the required reviews, create the PR (from a branch that follows the
<yourname>/description
naming convention) and then add the following PR comment:Additional notes