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

Bring ruby manual instrumentation docs up to par with rest of languages #1994

Merged
merged 12 commits into from Nov 16, 2022

Conversation

cartermp
Copy link
Contributor

@cartermp cartermp commented Nov 12, 2022

Ready for review!

Preview link: https://deploy-preview-1994--opentelemetry.netlify.app/docs/instrumentation/ruby/manual/

App I used to validate things: https://github.com/cartermp/otel-lang-samples/blob/main/ruby/app/app/controllers/app_controller.rb (sent traces to honeycomb)

Remaining:

  • Span status
  • Record exception
  • Check all code samples because there's sure as shit some stuff that's wrong

@cartermp cartermp marked this pull request as ready for review November 12, 2022 17:53
@cartermp cartermp requested review from a team as code owners November 12, 2022 17:53
Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice works, I am not a ruby expert, but the text looks good to me :)

@svrnm svrnm added the sig:ruby label Nov 14, 2022
Copy link
Contributor

@ahayworth ahayworth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it looks pretty good, and I'm going to 👍 it. I left some comments but none of them are technically blocking:

  • A lot of little style nits: indentation, stuff with parens, etc
  • At least one actual syntax error (although I don't truly think that's a Big Problem given that it's just an example in the docs)
  • Some comments about the in_span helper we provide and whether or not it should be mentioned alongside the more manual ways to do things.

Please feel free to take or leave them! ❤️

content/en/docs/instrumentation/ruby/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/ruby/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/ruby/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/ruby/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/ruby/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/ruby/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/ruby/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/ruby/manual.md Outdated Show resolved Hide resolved
content/en/docs/instrumentation/ruby/manual.md Outdated Show resolved Hide resolved
@cartermp
Copy link
Contributor Author

Thanks @ahayworth for the thorough review - I've addressed everything and will merge. Will also follow up on the linked issue I filed around explaining in_span a little better.

@cartermp cartermp merged commit d4598b0 into open-telemetry:main Nov 16, 2022
@cartermp cartermp deleted the cartermp/ruby-manual branch November 16, 2022 17:55
@ahayworth
Copy link
Contributor

Sounds good to me! 😄

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

Successfully merging this pull request may close these issues.

None yet

3 participants