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

Update getting-started.md with Bandit #4518

Merged
merged 7 commits into from
May 23, 2024

Conversation

pdgonzalez872
Copy link
Contributor

No description provided.

@pdgonzalez872 pdgonzalez872 requested review from a team as code owners May 21, 2024 18:46
Copy link

linux-foundation-easycla bot commented May 21, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@svrnm
Copy link
Member

svrnm commented May 22, 2024

@pdgonzalez872 thanks for raising this PR! Can you provide some details on the background for this PR? From my point of view the getting started should only make use of one webserver and then exercise the whole example with it. In other languages we have a section at the top that calls out that instrumentation is available for other languages, we can add this for sure.

@tsloughter
Copy link
Member

@svrnm The framework Phoenix only in the last release changed its default to the web server Bandit. So I think this was meant to cover the case for users on the latest Phoenix as well as still on previous versions who are looking to add OpenTelemetry.

Maybe just leaning in on the latest makes sense though.

@svrnm
Copy link
Member

svrnm commented May 22, 2024

Thanks @tsloughter that helps! Then this change makes sense.

Co-authored-by: Severin Neumann <[email protected]>
@pdgonzalez872
Copy link
Contributor Author

@tsloughter is spot on.

I'd still try to have both the old and the new web servers as options if possible. Even though it's the new default, it is still somewhat rare. The docs, especially, the getting started, should help folks hit the ground running, that's why I went the old/new approach.

WDYT?

@svrnm
Copy link
Member

svrnm commented May 22, 2024

/easycla

@svrnm
Copy link
Member

svrnm commented May 22, 2024

works for me, if someone from @open-telemetry/erlang-approvers approves this PR we can do that.

@cartermp
Copy link
Contributor

/fix:all

@opentelemetrybot
Copy link
Contributor

You triggered fix:all action run at https://github.com/open-telemetry/opentelemetry.io/actions/runs/9208653847

@pdgonzalez872
Copy link
Contributor Author

hey, that's a cool check!
image

fixed

@cartermp cartermp merged commit a08b8e5 into open-telemetry:main May 23, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants