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

profiler: enable execution traces by default for go1.21+ #2226

Merged

Conversation

felixge
Copy link
Member

@felixge felixge commented Sep 21, 2023

What does this PR do?

Enable execution traces by default for go1.21+

Motivation

We've verified that this is safe and has no performance impact on our internal fleet. Let's allow all customers to enjoy this feature without having to figure out how to turn it on.

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.

For Datadog employees:

  • If this PR touches code that handles credentials of any kind, such as Datadog API keys, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@felixge felixge requested a review from a team as a code owner September 21, 2023 08:07
@pr-commenter
Copy link

pr-commenter bot commented Sep 21, 2023

Benchmarks

Benchmark execution time: 2023-09-27 11:36:04

Comparing candidate commit 3cca387 in PR branch felix.geisendoerfer/PROF-8240-enable-execution-tracing-by-default with baseline commit 6a938b3 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 40 metrics, 1 unstable metrics.

Copy link
Contributor

@nsrip-dd nsrip-dd left a comment

Choose a reason for hiding this comment

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

Thanks!

@felixge
Copy link
Member Author

felixge commented Sep 21, 2023

I'm waiting to merge this until another commit lands as I want to try out another merge queue thing for the document I'm writing about it.

@felixge
Copy link
Member Author

felixge commented Sep 26, 2023

/merge

@dd-devflow
Copy link

dd-devflow bot commented Sep 26, 2023

🚂 MergeQueue

Pull request added to the queue.

This build is going to start soon! (estimated merge in less than 9m)

you can cancel this operation by commenting your pull request with /merge -c!

@dd-devflow
Copy link

dd-devflow bot commented Sep 26, 2023

🚨 MergeQueue

not able to merge the branch in the target branch

Details

Error: PUT https://api.github.com/repos/DataDog/dd-trace-go/pulls/2226/merge: 405 5 of 5 required status checks are expected. []

FullStacktrace:
activity error (type: github.GithubService_MergePullRequest, scheduledEventID: 41, startedEventID: 42, identity: 1@github-worker-8496dd8b5f-jppjp@): PUT https://api.github.com/repos/DataDog/dd-trace-go/pulls/2226/merge: 405 5 of 5 required status checks are expected. [] (type: ErrorResponse, retryable: true)

If you need support, contact us on slack #ci-interfaces with those details!

@felixge felixge enabled auto-merge (squash) September 27, 2023 11:21
@felixge felixge merged commit 04c819d into main Sep 27, 2023
52 checks passed
@felixge felixge deleted the felix.geisendoerfer/PROF-8240-enable-execution-tracing-by-default branch September 27, 2023 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants