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

Generate docs with ExDoc #268

Merged
merged 8 commits into from
Sep 26, 2021

Conversation

kw7oe
Copy link
Contributor

@kw7oe kw7oe commented Aug 21, 2021

This PR add ability to generate documentation for opentelemetry and opentelemetry_exporter with ExDoc. This work is mostly refer to the following resources:

So far, there are a few caveats:

  • I haven't thoroughly check and compare the generated docs for each modules and functions with the current docs.
  • Not sure if the changes will work for publishing to Hex.pm and what are the workflow like. ExDoc README linked above does document the manual steps required.
  • The $version used in docs.sh is hardcoded and used by both opentelemetry and opentelemetry_exporter.

Since I'm quite new to Erlang and Rebar, so there are probably places where I did things incorrectly or can be further improve.

To test out locally:

  • Make sure you're running on OTP 24+
  • Install ExDoc with `mix escript.install github elixir-lang/ex_doc
  • Run ./docs.sh.
  • View the generated HTML by opening:
    • apps/opentelemetry/doc/index.html
    • apps/opentelemetry_exporter/doc/index.html

Screenshot

OpenTelemetry

Screenshot 2021-08-21 at 6 19 54 PM
Screenshot 2021-08-21 at 6 19 59 PM
Screenshot 2021-08-21 at 6 20 04 PM

OpenTelemetry Exporter

Screenshot 2021-08-21 at 6 20 34 PM
Screenshot 2021-08-21 at 6 20 39 PM

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 21, 2021

CLA Signed

The committers are authorized under a signed CLA.

@kw7oe kw7oe marked this pull request as ready for review August 21, 2021 10:32
@kw7oe kw7oe requested a review from a team August 21, 2021 10:32
@codecov
Copy link

codecov bot commented Aug 21, 2021

Codecov Report

Merging #268 (cf1b218) into main (4905c93) will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #268      +/-   ##
==========================================
- Coverage   36.58%   36.55%   -0.04%     
==========================================
  Files          45       45              
  Lines        3187     3187              
==========================================
- Hits         1166     1165       -1     
- Misses       2021     2022       +1     
Flag Coverage Δ
api 62.44% <ø> (ø)
elixir 15.91% <ø> (ø)
erlang 36.50% <ø> (-0.04%) ⬇️
exporter 19.75% <ø> (ø)
sdk 78.94% <ø> (-0.19%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ntelemetry_exporter/src/opentelemetry_exporter.erl 77.03% <ø> (ø)
apps/opentelemetry/src/otel_resource_detector.erl 91.42% <0.00%> (-1.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4905c93...cf1b218. Read the comment docs.

docs.sh Outdated Show resolved Hide resolved
@tsloughter
Copy link
Member

Awesome! Looking great.

I noticed edoc markup was replaced with html like when making a list. I thought it would be markdown based on examples I'd seen from @wojtekmach. Maybe that is a separate thing and I'm conflating ways of generating ex_doc from Erlang? If so maybe it'd be simple to update this PR to use the chunks based approach like https://github.com/wojtekmach/dotfiles/blob/625d2daba1392e82dab9681d23da09efb2a3fd0f/bin/docs_chunk.exs

@wojtekmach
Copy link
Contributor

wojtekmach commented Aug 21, 2021

@tsloughter yeah, sorry for causing confusion, i had a bunch of different proof of concepts for various aspects of docs generation that I was spamming you all about.

This PR follows https://github.com/elixir-lang/ex_doc#using-exdoc-with-erlang-projects and it is definitely the way to go.

Co-authored-by: Tristan Sloughter <t@crashfast.com>
@kw7oe
Copy link
Contributor Author

kw7oe commented Aug 22, 2021

For the HTML markup, I'm following here: https://github.com/beam-telemetry/telemetry/blob/f559faee2f01f0757ac055e501f48abc8e1ba1bd/src/telemetry.erl#L122 as the current markdown based doesn't work as expected when generating with ExDoc for Erlang docs.

Just curious, @wojtekmach is that the intended behaviour (some markdown is not supported) when using ExDoc to genearte Erlang docs? Is it right to say that the markup currently supported is as documented here: https://erlang.org/doc/apps/edoc/chapter.html#wiki-notation?

@wojtekmach
Copy link
Contributor

Is it right to say that the markup currently supported is as documented here: https://erlang.org/doc/apps/edoc/chapter.html#wiki-notation?

Yes, it is correct.

To be specific, ExDoc doesn’t care what you use to write your docs as long as these two things are true:

  1. The docs are written to the Docs chunk
  2. The Docs chunk uses either text/markdown or the application/erlang+html content type

The ExDoc Erlang instructions show how to set your project to extract docs into chunk using Edoc. And then how to trigger ex_doc escript. If you use something else to write docs to the chunk, ExDoc doesn’t care.

hope this helps, please lmk otherwise.

@kw7oe
Copy link
Contributor Author

kw7oe commented Aug 29, 2021

Yes, it is correct.

To be specific, ExDoc doesn’t care what you use to write your docs as long as these two things are true:

1. The docs are written to the Docs chunk

2. The Docs chunk uses either text/markdown or the application/erlang+html content type

The ExDoc Erlang instructions show how to set your project to extract docs into chunk using Edoc. And then how to trigger ex_doc escript. If you use something else to write docs to the chunk, ExDoc doesn’t care.

hope this helps, please lmk otherwise.

Ahh okay, that's very clear. Thanks for the input 👍🏻

@kw7oe
Copy link
Contributor Author

kw7oe commented Sep 16, 2021

@wojtekmach Just curious, if we were to support Markdown syntax in edoc, that would mean that the work need to be done on Erlang edoc itself right?

If that's the case, is there any ongoing work related to that? Let me know if you prefer to have this discussion on a different channel.

@tsloughter Also, I just checked the generated docs again and found out that some modules is not generated by ExDoc because of the lack of module level documentation. E.g. otel_utils.erl.

Are those modules intended to be private and this behaviour is expected?

@tsloughter
Copy link
Member

@kw7oe sorry, I had missed your question. If a module is private it will be marked as @hidden. That should be the only reason docs are skipped. Pretty sure edoc doesn't skip modules just because they are missing doc headers... Which is why it has the @hidden option. I'd hope ex_doc could do the same.

But, hehe, otel_utils probably should be marked as @hidden :)

I was going to merge this and any updates can come in future PRs? Did you ever hear back from @wojtekmach

@kw7oe
Copy link
Contributor Author

kw7oe commented Sep 26, 2021

@kw7oe sorry, I had missed your question. If a module is private it will be marked as @hidden. That should be the only reason docs are skipped. Pretty sure edoc doesn't skip modules just because they are missing doc headers... Which is why it has the @hidden option. I'd hope ex_doc could do the same.

But, hehe, otel_utils probably should be marked as @hidden :)

I was going to merge this and any updates can come in future PRs? Did you ever hear back from @wojtekmach

@tsloughter Yeap, edoc doesn't skip but ex_doc did, and here's the ongoing PR: erlang/otp#5205 that would probably fix that.

Yeap all good to merge and any new updates can come in the future PRs. I haven't heard back from wojtekmach but it's fine since it seems like he's on vacation.

@tsloughter tsloughter merged commit 9565e4f into open-telemetry:main Sep 26, 2021
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.

3 participants