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

fix: only take the first item in a comma-separated list for pg attrs #142

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

ahayworth
Copy link
Contributor

@ahayworth ahayworth commented Oct 11, 2022

In a recent upstream PR1, a change was made that introduced the
possibility of the conninfo_hash containing a comma-separated list if
the hostname we connect to resolves to multiple IP addresses.

In the tests, we say to connect to localhost: and sure enough, that
resolves to both 127.0.0.1 and ::1. 🤦

I considered simply relaxing the validation here, but we'd break
semantic conventions: net.peer.port must be an integer, not a string -
so we can't just pass through the unmodified values. The values will
always be duplicate, comma-separated strings however: the actual
difference shows up in net.peer.ip.

Anyways, we add a helper function that tries to be smart about not doing
more work than is necessary.

Footnotes

  1. https://github.com/ged/ruby-pg/pull/485

In a recent upstream PR[^1], a change was made that introduced the
possibility of the `conninfo_hash` containing a comma-separated list if
the hostname we connect to resolves to multiple IP addresses.

In the tests, we say to connect to `localhost`: and sure enough, that
resolves to both `127.0.0.1` and `::1`. 🤦

I considered simply relaxing the validation here, but we'd break
semantic conventions: `net.peer.port` must be an integer, not a string -
so we can't just pass through the unmodified values. The values will
always be duplicate, comma-separated strings however: the actual
difference shows up in `net.peer.ip`.

Anyways, we add a helper function that tries to be smart about not doing
more work than is necessary.
@ahayworth
Copy link
Contributor Author

To make it a little more clear: if you pass localhost as the connection host, then conninfo_hash looks like:

host: localhost,localhost
port: 5432,5432
address: 127.0.0.1, ::1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants