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

WIP: contrib/jackc/pgx.v5: add support for pgx #2236

Closed
wants to merge 14 commits into from

Conversation

rarguelloF
Copy link
Contributor

@rarguelloF rarguelloF commented Sep 27, 2023

What does this PR do?

Continuation of #2160

Motivation

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!

package pgx

import (
"context"

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
File is not gci-ed with --skip-generated -s standard,prefix(gopkg.in/DataDog/dd-trace-go.v1),default (gci)

import (
"context"
"fmt"

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
File is not gci-ed with --skip-generated -s standard,prefix(gopkg.in/DataDog/dd-trace-go.v1),default (gci)

os.Exit(m.Run())
}

func assertCommonTags(t *testing.T, s mocktracer.Span, qType string) {

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
unused-parameter: parameter 't' seems to be unused, consider removing or renaming it as _ (revive)

assert.Equal(t, "parent", s2.Tag(ext.ResourceName))
}

func TestTraceBatch(t *testing.T) {

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
unused-parameter: parameter 't' seems to be unused, consider removing or renaming it as _ (revive)

@pr-commenter
Copy link

pr-commenter bot commented Sep 27, 2023

Benchmarks

Benchmark execution time: 2023-11-16 16:56:21

Comparing candidate commit 0c65ee8 in PR branch rarguelloF/support-pgx with baseline commit 6ae85ec in branch main.

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

@github-actions
Copy link

This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale Stuck for more than 1 month label Oct 18, 2023
@jalaziz
Copy link

jalaziz commented Oct 30, 2023

What's left to get this over the line? Would love support for pgx.

@github-actions github-actions bot removed the stale Stuck for more than 1 month label Oct 31, 2023
@sejin-P
Copy link

sejin-P commented Nov 7, 2023

Is there any other remained process?

@Eyal-Shalev
Copy link

@jalaziz

What's left to get this over the line? Would love support for pgx.

@sejin-P

Is there any other remained process?

I see that tests are failing

@sgtsquiggs
Copy link

this is missing several features for parity with database/sql:

  • DBMPropagation
    • very difficult the way pgx is currently written
    • adding TraceQueryInjectComment (etc...) on the pgx side could resolve this
  • many relevant config options
    • WithServiceName
    • WithAnalytics
    • WithAnalyticsRate
    • WithChildSpansOnly
    • WithErrorCheck
    • WithCustomTag
    • WithDBMPropagation (see above)

Copy link

This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale Stuck for more than 1 month label Dec 11, 2023
Copy link

This PR was closed because it has been open for 30 days with no activity.

@github-actions github-actions bot closed this Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stuck for more than 1 month
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants