Djt/0311/merge2

[djthorpe]

No description provided.

[Copilot]

Pull request overview

This PR modernizes tracing/logging across the pg pool and queue runner, adds a targeted connection option, and updates module dependencies/tooling.

Changes:

• Refactors pg pool tracing setup (TraceFn and OpenTelemetry) and removes exported tracer constructors.
• Updates queue runtime logging to use log/slog via a context-injected logger, with additional task/error logging and status checking.
• Adds pg.WithPassword plus tests, updates README examples, and bumps Go/module dependencies.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tracer.go	Removes exported tracer constructors; keeps query tracing implementation.
pool.go	Copies tracer config into a standalone allocation and wires tracing into pgx config.
opts.go	Embeds tracer by value, adds WithPassword, expands accepted URL schemes, updates trace options wiring.
opts_test.go	Adds test coverage for WithPassword behavior and precedence.
pkg/queue/run.go	Switches to slog logger via context, adds task error logging and status validation, tweaks span naming.
cmd/pgqueue/main.go	Switches CLI logging to *slog.Logger and configures handler/level selection.
cmd/pgqueue/server.go	Updates logging calls to slog, modifies middleware setup, changes TLS loading, wires queue logger via context.
cmd/pgmanager/server.go	Changes TLS loading to read cert/key bytes before calling TLSConfig.
README.md	Documents WithTracer/WithPassword and updates code examples for current APIs.
go.mod / go.sum	Bumps Go version and upgrades multiple dependencies (including OTel).
etc/embed.go	Removes embedded frontend asset FS.

Comments suppressed due to low confidence (3)

tracer.go:49

• This PR removes the exported constructors NewTracer and NewOTELTracer. Even if they were rarely used internally, removing exported symbols is a breaking API change for downstream users. Consider keeping them (possibly deprecated) or providing a migration path (e.g., exported options-only API) before removing.

//////////////////////////////////////////////////////////////////////////////
// PUBLIC METHODS

func (t *tracer) TraceQueryStart(ctx context.Context, _ *pgx.Conn, data pgx.TraceQueryStartData) context.Context {

cmd/pgqueue/server.go:160

• result is updated from this goroutine concurrently with the manager goroutine above, without synchronization, causing a data race. Protect result with a mutex (or use errgroup.Group / an error channel) when joining errors.

		if err := server.Run(ctx.ctx); err != nil {
			if !errors.Is(err, context.Canceled) {
				result = errors.Join(result, err)
			}
			ctx.cancel()

cmd/pgqueue/server.go:100

• The comment says "Set logging middleware", but middleware is initialized empty and no logging middleware is added anymore (only optional OTEL). Either add a replacement request-logging middleware compatible with *slog.Logger, or update/remove the comment to reflect the current behavior.

	// Set logging middleware
	middleware := httphandler.HTTPMiddlewareFuncs{}

	// If we have an OTEL tracer, add tracing middleware
	if ctx.tracer != nil {
		middleware = append(middleware, otel.HTTPHandlerFunc(ctx.tracer))
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The file imports the same package path twice (github.com/mutablelogic/go-pg/pkg/queue) under two names (manager and queue), which will not compile. Consider importing it once (e.g., as queue) and renaming the local manager variable to something like mgr so you can call queue.WithLogger/queue.New without shadowing issues.

Suggested change

manager "github.com/mutablelogic/go-pg/pkg/queue"

[Copilot]

result is written from this goroutine without synchronization, while another goroutine also writes to result below. This introduces a data race. Use a mutex around result = errors.Join(...), or switch to errgroup.Group / a buffered error channel to collect errors safely.

[Copilot]

This log line includes the full *schema.Task object. Because Task implements String() via JSON marshaling, this can log the entire payload/result (potentially large and/or sensitive). Consider logging only minimal identifiers (e.g., task.Id, task.Queue, task.Namespace) and avoid logging raw payloads by default.

Suggested change

	log.ErrorContext(ctx, "task error", "task", task, "err", err)
	log.ErrorContext(ctx, "task error",
	"task_id", task.Id,
	"task_queue", task.Queue,
	"task_namespace", task.Namespace,
	"err", err,
	)

[Copilot]

The comment says "log a warning" when status != "released", but the code actually turns it into a returned error via errors.Join. Either log a warning (and keep returning result), or update the comment/behavior so callers aren't unexpectedly treated as failing due solely to a non-"released" status.

Suggested change

	result = errors.Join(result, fmt.Errorf("task status: %s", status))
	slog.WarnContext(child2, "task not released", "task_id", task.Id, "status", status)