Skip to content
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

Golang support #136

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Golang support #136

wants to merge 2 commits into from

Conversation

hakoerber
Copy link

@hakoerber hakoerber commented Sep 20, 2024

What does this PR do?

Add support for Golang! (#128)

Motivation

I have some Golang lambdas managed with AWS SAM and would like to use the DD macro with them ;)

Testing Guidelines

I added tests for the relevant language-specific functions (mainly layer.ts). Most functionality is language agnostic (and current tests mainly use python), so I added no go-specific tests here.

I also found some tests (but no matching implementation) for the go1.10 runtime. Using that runtime is deprecated, so I skipped implementation for that.

Additional Notes

It is a bit weird to detect that a lambda function is using go, as there is nothing inherent in the SAM template that identifies a lambda as "written in Go", due to it being a compiled language.

It is not possible to use the Runtime configuration, as it's currently being done for the other languages. Runtime is now always supposed to be set to provided.al* for go, which is also used for other compiled runtimes (e.g. Rust).

My approach:

  • I first though about providing a macro-level configration value to set the lambda for all discovered runtimes, but this will not work with multiple lambdas with different runtimes.
  • Using the Metadata.BuildMethod (which is often set to go1.x for go lambdas) is also not reliable, as this may also be a language-agnostic makefile.
  • I did not want to "pollute" either the function tags or the env variables with a new variable that specifies the runtime.

In the end, I decided to add another Metadata field, Metadata.Runtime, that will treat the lambda as a Golang lambda. It's backwards compatible, so all current configuration will continue to use the runtime value if applicable, regardless of Metadata.Runtime.

Funny thing is: There is nothing inherent to Go about all of this. There is no Golang tracing layer anyway, like it exists for the other languages. In the end, we just add the Datadog base layer and that's it. So it would have been possible to make this completely independent of go, and just add the Database base layer whenever we see a lambda with one of the provided.al* runtimes. That would make it work for other compiled languages as well (e.g. Rust). I decided against it, as language support should be specific in my opinion, but this is up to debate of course.

In the end, there are two ways to proceed, and I think this is maintainer-level decision:

  1. Keep the PR as it is, only officially support Go
  2. Add (implicit) support for all compiled languages by removing the go-specific stuff

I set this PR to "draft", as documentation is missing. I will update this PR with documentation when the decision about the question above (and possible rework) is done.

Types of changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • ⚠️ This PR impacts documentation, and it has been updated (or a ticket has been logged) => will update docs when finalized
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog

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.

1 participant