Default metrics pool name to Application Name from connection string#6533
Default metrics pool name to Application Name from connection string#6533
Conversation
When no explicit NpgsqlDataSourceBuilder.Name is set, default the data source name (used as db.client.connection.pool.name in metrics) to the Application Name from the connection string, falling back to the connection string itself only if Application Name is also unset. Closes #6531 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR changes how NpgsqlDataSource.Name is derived when no explicit NpgsqlDataSourceBuilder.Name is provided, so that the db.client.connection.pool.name metric tag defaults to the connection string’s Application Name (and falls back to the password-scrubbed connection string when Application Name is unset).
Changes:
- Update
NpgsqlDataSourcedefault naming fallback to prefersettings.ApplicationName. - Add a metrics test verifying the new
Application Namefallback behavior. - Adjust the password-leak test to cover the connection-string fallback path (no
Application Nameset).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Npgsql/NpgsqlDataSource.cs |
Changes the default Name derivation chain to include ApplicationName before falling back to the (password-scrubbed) connection string. |
test/Npgsql.Tests/MetricTests.cs |
Adds/updates tests for the new pool-name fallback behavior and password scrubbing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (settings.PersistSecurityInfo) | ||
| { | ||
| ConnectionString = settings.ToString(); | ||
|
|
||
| // The data source name is reported in tracing/metrics, so avoid leaking the password through there. | ||
| Name = name ?? settings.ToStringWithoutPassword(); | ||
| Name = name ?? settings.ApplicationName ?? settings.ToStringWithoutPassword(); | ||
| } | ||
| else | ||
| { | ||
| ConnectionString = settings.ToStringWithoutPassword(); | ||
| Name = name ?? ConnectionString; | ||
| Name = name ?? settings.ApplicationName ?? ConnectionString; |
There was a problem hiding this comment.
settings.ApplicationName can be an empty string (e.g., connection string contains Application Name=). With the current null-coalescing chain, an empty application name becomes the data source Name, resulting in an empty db.client.connection.pool.name tag and skipping the intended fallback to the (password-scrubbed) connection string. Consider treating empty/whitespace application names as unset (e.g., use a length/IsNullOrEmpty check before selecting it).
| var applicationName = "MetricsDataSource" + Interlocked.Increment(ref _dataSourceCounter); | ||
| var dataSourceBuilder = base.CreateDataSourceBuilder(); | ||
| dataSourceBuilder.ConnectionStringBuilder.ApplicationName = applicationName; | ||
| // Do not set the data source name - this makes the pool name default to the Application Name | ||
| await using var dataSource = dataSourceBuilder.Build(); |
There was a problem hiding this comment.
This test uses an instance-level counter to generate applicationName. Since the test assembly is parallelizable, different MetricTests instances can run concurrently and produce the same MetricsDataSource1 name, which can cause metric aggregation across tests (same db.client.connection.pool.name) and lead to flaky assertions. Use a process-wide unique value (e.g., a static counter, Guid, or include TestContext.CurrentContext.Test.ID) to avoid collisions under parallel execution.
When no explicit
NpgsqlDataSourceBuilder.Nameis set, thedb.client.connection.pool.namemetric tag currently defaults to the full connection string, making metrics hard to distinguish in monitoring dashboards. This change defaults it to theApplication Namefrom the connection string instead, falling back to the connection string only ifApplication Nameis also unset.The fallback priority is now:
NpgsqlDataSourceBuilder.Name(unchanged)ApplicationNamefrom the connection string (new)This is done by inserting
settings.ApplicationNamein theNameassignment chain inNpgsqlDataSource.cs. No new connection string property is needed.Tests:
Pool_name_defaults_to_application_nameverifying the new fallback behaviorPassword_does_not_leak_via_datasource_nameto test the connection-string fallback path (noApplicationNameset)Closes #6531