opentelemetry-rust icon indicating copy to clipboard operation
opentelemetry-rust copied to clipboard

Main readme to have runnable example

Open cijothomas opened this issue 1 year ago • 9 comments

I am not sure if we should maintain a lengthy code in the main readme.. If we chose to, we need to make sure it is showing all signals, and is a runnable example.

(Personally, I am okay to remove this from main readme file, but open to suggestions)

cijothomas avatar May 15 '24 19:05 cijothomas

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 74.1%. Comparing base (595ad05) to head (3f97f94). Report is 89 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1778   +/-   ##
=====================================
  Coverage   74.1%   74.1%           
=====================================
  Files        125     125           
  Lines      19481   19481           
=====================================
+ Hits       14452   14454    +2     
+ Misses      5029    5027    -2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 15 '24 19:05 codecov[bot]

The example could be bit overwhelming for newcomers due to its length. Will it make sense to split it in 3 relatively smaller examples for logs, metrics and traces ?

lalitb avatar May 20 '24 16:05 lalitb

The example could be bit overwhelming for newcomers due to its length. Will it make sense to split it in 3 relatively smaller examples for logs, metrics and traces ?

Probably can link to smaller examples? I feel like if we split it up we will just repeat a lot of setups

TommyCpp avatar May 20 '24 16:05 TommyCpp

The example could be bit overwhelming for newcomers due to its length. Will it make sense to split it in 3 relatively smaller examples for logs, metrics and traces ?

I am not sure what would be a good way... Keeping 3 examples will make the main readme even longer.. The opentelemetry-stdout has single example, but split by signal feature...

Maybe we should steal something like https://github.com/tokio-rs/axum/tree/main/examples/readme ? We can keep an example in the examples directory, named readme, which can be referred from the readme.md file. It can ensure main readme.md file is still small, and example is always kept up-to-date...

cijothomas avatar May 20 '24 16:05 cijothomas

Marking to draft, as I need to decide if we can do any better than this.. Please keep sharing the feedback.

cijothomas avatar May 20 '24 16:05 cijothomas

and example is always kept up-to-date

I don't think that's a small task given we are improving APIs on a daily basis. I'd rather we get a stable place first and then add more examples. We cut down lots of examples earlier to keep the maintaince burdan low

TommyCpp avatar May 20 '24 16:05 TommyCpp

I feel like if we split it up we will just repeat a lot of setups

We can a common toml for all 3 examples. Apart from that setup would be different in all three.

lalitb avatar May 20 '24 16:05 lalitb

and example is always kept up-to-date

I don't think that's a small task given we are improving APIs on a daily basis. I'd rather we get a stable place first and then add more examples. We cut down lots of examples earlier to keep the maintaince burdan low

We are already keeping example in stdout/otlp up-to-date! (it is also a good way for PR reviewers to see the impact of breaking changes, as we ourselves show the changes required in the example. Given most changelogs link to PR#, it also helps end users to know what changes they must do in their own project).

Not very sustainable path, but once we announce stable release, this should be resolved. Until then a lot of users will be 😞.

cijothomas avatar May 20 '24 16:05 cijothomas

and example is always kept up-to-date

I don't think that's a small task given we are improving APIs on a daily basis. I'd rather we get a stable place first and then add more examples. We cut down lots of examples earlier to keep the maintaince burdan low

We are already keeping example in stdout/otlp up-to-date! (it is also a good way for PR reviewers to see the impact of breaking changes, as we ourselves show the changes required in the example. Given most changelogs link to PR#, it also helps end users to know what changes they must do in their own project).

Not very sustainable path, but once we announce stable release, this should be resolved. Until then a lot of users will be 😞.

Just to be clear I am not against the idea of breaking changes on APIs now or adding more examples. My worries are given we haven't stable yet, adding too many examples will add unnecessary burdan on us as maintainers, as well as contributors.

TommyCpp avatar May 20 '24 17:05 TommyCpp

Closing old, inactive PRs. Feel free to reopen when ready. Thanks.

cijothomas avatar Jul 31 '24 18:07 cijothomas