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.

Sign up for GitHub

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

Jump to bottom

chore: add sc to ecto #142

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore: add sc to ecto #142

wants to merge 1 commit into from

Conversation

yordis
Copy link
Contributor

@yordis yordis commented Dec 18, 2022

No description provided.

@yordis yordis force-pushed the add-sc-to-ecto branch 5 times, most recently from 7635fe1 to fc1249d Compare December 18, 2022 19:20
@yordis yordis changed the title chore: add sc to oban chore: add sc to ecto Dec 20, 2022
@yordis yordis force-pushed the add-sc-to-ecto branch 2 times, most recently from 2c143d0 to c733daf Compare December 20, 2022 19:06
@tsloughter
Copy link
Member

Argh. I seemed to have missed this. It now has conflicts and I think will conflict with #65

Trying to get these Ecto PRs all merged but they all have a little bit I'm unsure about.

@yordis
Copy link
Contributor Author

yordis commented Jul 7, 2023

We should try to convince the adapter authors and Ecto to follow some rules to setup such a key.

Although the alternative PR will work, the direction of dependency where OTEL is aware of ALL things out there is brutal.

Maybe the Ecto team could support it a bit and deprecate that way eventually.

@tsloughter
Copy link
Member

@yordis oh and this is still a draft. Is it still not ready?

@yordis
Copy link
Contributor Author

yordis commented Jul 8, 2023

I mean, kind of yeah. I would probably try to get as many attributes as possible but it seems that #65 has some good stuff there also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bryannaegele bryannaegele Awaiting requested review from bryannaegele bryannaegele will be requested when the pull request is marked ready for review bryannaegele is a code owner

@tsloughter tsloughter Awaiting requested review from tsloughter tsloughter will be requested when the pull request is marked ready for review tsloughter is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
elixir instrumentation opentelemetry_ecto
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yordis @tsloughter