opentelemetry-erlang-contrib icon indicating copy to clipboard operation
opentelemetry-erlang-contrib copied to clipboard

Made 'db.url' more complete and some minor refactor

Open spencerdcarlson opened this issue 2 years ago • 4 comments

Normalize Ecto URI and Config the same way that ecto does it here

Normalizing the URI allows for:

  • db.url to be more complete
  • net.peer.name to be added conditionally
  • net.peer.port to be added conditionally

Using Ecto.Repo.Supervisor.parse_url/1 to normalize the ecto URI requires that ecto is available at runtime so I added the oldest version of ecto that has what we need (v0.15.0). This seems like a reasonable dependency to add given that these are ecto metrics that are being collected.

spencerdcarlson avatar Jul 22 '22 02:07 spencerdcarlson

This is looking good now, just 2 suggestions from @bryannaegele and some conflicts to resolve @spencerdcarlson

tsloughter avatar Dec 15 '22 00:12 tsloughter

@tsloughter I believe I resolved all conflicts.

spencerdcarlson avatar Dec 15 '22 02:12 spencerdcarlson

@tsloughter any update on this?

spencerdcarlson avatar Jan 04 '23 16:01 spencerdcarlson

@spencerdcarlson there are still a few outstanding requests that have not been addressed, particularly around adding ecto as a dep and the extra processing around URI strings. Once those have been addressed, I can do a final review with final suggestions/changes.

bryannaegele avatar Jan 04 '23 16:01 bryannaegele