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

contrib: add go.mod to isolate contribs gqlgen, httptreemux, go-elasticsearch, cloud.google.com/go/pubsub.v1, hashicorp/consul & hashicorp/vault #2358

Merged
merged 22 commits into from Nov 22, 2023

Conversation

darccio
Copy link
Contributor

@darccio darccio commented Nov 15, 2023

What does this PR do?

Isolates the following contribs as independent modules:

  • 99designs/gqlgen
  • dimfeld/httptreemux.v5
  • elastic/go-elasticsearch.v6
  • cloud.google.com/go/pubsub.v1
  • hashicorp/consul
  • hashicorp/vault

This will be done for each contrib, and this is a first PR to validate the approach. Next PRs will consist of multiple contribs.

Motivation

Security scanners yield false positives, informing vulnerable versions that are present in the go.mod but not compiled into the final binary.

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!

@darccio darccio requested a review from a team November 15, 2023 11:47
@darccio darccio requested a review from a team as a code owner November 15, 2023 11:47
@github-actions github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Nov 15, 2023
@darccio darccio requested review from a team as code owners November 20, 2023 13:53
@darccio darccio force-pushed the dario.castane/AIT-8717/isolate-contribs-batch-1 branch from bb1a5da to 7e4d418 Compare November 20, 2023 13:59
@darccio darccio force-pushed the dario.castane/AIT-8717/isolate-contribs-batch-1 branch from 81985fa to afffd7c Compare November 20, 2023 14:56
@pr-commenter
Copy link

pr-commenter bot commented Nov 20, 2023

Benchmarks

Benchmark execution time: 2023-11-22 12:03:50

Comparing candidate commit 6848485 in PR branch dario.castane/AIT-8717/isolate-contribs-batch-1 with baseline commit d9ad3a9 in branch v2-dev.

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

@RomainMuller
Copy link
Contributor

Makes me wonder if this should also add a go.work file at the repository root referencing all those modules (and the root itself), for improved IDE experience?

@darccio
Copy link
Contributor Author

darccio commented Nov 22, 2023

Makes me wonder if this should also add a go.work file at the repository root referencing all those modules (and the root itself), for improved IDE experience?

Good proposal. I'll review it.

@darccio darccio changed the title contrib: add go.mod to isolate contribs gqlgen, gomemcache, database/sql, httptreemux & go-elasticsearch contrib: add go.mod to isolate contribs gomemcache, httptreemux & go-elasticsearch Nov 22, 2023
@darccio darccio changed the title contrib: add go.mod to isolate contribs gomemcache, httptreemux & go-elasticsearch contrib: add go.mod to isolate contribs gqlgen, httptreemux & go-elasticsearch Nov 22, 2023
@darccio
Copy link
Contributor Author

darccio commented Nov 22, 2023

@RomainMuller It seems that the current opinion about go.work is that they represent local paths that aren't shareable. Check PR #1778 for more detail.

@darccio darccio changed the title contrib: add go.mod to isolate contribs gqlgen, httptreemux & go-elasticsearch contrib: add go.mod to isolate contribs gqlgen, httptreemux, go-elasticsearch, cloud.google.com/go/pubsub.v1, hashicorp/consul & hashicorp/vault Nov 22, 2023
@darccio
Copy link
Contributor Author

darccio commented Nov 22, 2023

CI looks green and great:

image
image

@darccio
Copy link
Contributor Author

darccio commented Nov 22, 2023

As agreed, I'll merge this PR and next ones without CR as long as CI shows that the contribs' tests are running properly.

@darccio darccio merged commit 5ade6bb into v2-dev Nov 22, 2023
47 of 49 checks passed
@darccio darccio deleted the dario.castane/AIT-8717/isolate-contribs-batch-1 branch November 22, 2023 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:ecosystem contrib/* related feature requests or bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants