Envoy: Doxygen generation

[htuch]

We should add a "make doxygen" target and build with Doxygen. @mattklein123 already has some progress on this in docs/Doxyfile. It might also be worth investigating doing this in the CI and publishing.

@jamessynge

[dnoe]

I'll take this, and probably also clean up some tech debt related to things that could be Doxygen annotated but aren't right now (with strong emphasis only on public API components). For example the Http::AccessLog::ResponseFlag enum has some nice details on what each piece of the enum means but the comments aren't in Doxygen style.

[derekargueta]

@mattklein123 is this still wanted? If so I'd like to try this out. Also not seeing the mentioned Doxyfile in the description so starting from scratch unless that's still around somewhere

[mattklein123]

@derekargueta yes, would be cool. I think the random Doxyfile we had at one point got deleted, so I would just start from scratch.

[jmarantz]

I guess I have started working on this. Might as well assign to me.

[jmarantz]

https://gist.github.com/vidavidorra/548ffbcdae99d752da02 has some ideas on how to auto-deploy the generated doxygen to github pages.

[jmarantz]

not really making progress on this.

[markmandel]

I'm taking a stab at this - mostly as a way to learn my way around Envoy's codebase.

First pass is getting a Doxyfile working, so that if you have doxygen installed locally - you can gen some documentation. Long term would like this to be a ci target.

I will probably need some help making sure I've included everything in the documentation as well. Will keep you all up to date with how I go.

[markmandel]

So I basically have this working over on https://github.com/markmandel/envoy/tree/docs/doxygen (once https://github.com/envoyproxy/envoy-build-tools/pull/26 is approved).

Right now it's integrated with ./ci/run_envoy_docker.sh './ci/do_ci.sh docs'. It does generate ~800MB of documentation, so I don't know if you want it part of your docset that is generated on every build.

I'm happy to split it out into something like './ci/do_ci.sh doxygen' if people think that would be better?

Guidance would be appreciated :+1:

[jmarantz]

There's no envyproxy/envoy PR for this yet, right?

I had started trying to tackle Doxygen here: https://github.com/jmarantz/envoy/tree/doxygen . Here were the hurdles in my way:

• I did not find the output as useful as I was hoping, but you may have better luck
• It's easy to get a Doxygen run, and the Envoy codebase does have a lot of Doxygen-ish comments. But given the wide number of code-writers and no current mechanism to ensure the Doxygen comments are formatted properly, my observation was that significant number of them were not valid, or were missing altogether.

It would be really great though to get this all done, particularly if current 'master' can have a published doc-link, kept up-to-date automatically.

In my opinion what's needed is a multi-PR effort where we wind up having invalid or missing Doxygen comments result in a format-test failure. But before you get there we need to have a multi-PR effort to correct the current codebase. To avoid falling behind during that process I think we'd need a check_format rule where any file you touch must be doxygen-correct and complete. Hopefully will spur crowdsourcing of this large effort.

@envoyproxy/maintainers WDYT of that strategy?

One other point: In a past project we used a very ugly bash script to doxygen-ify code that did not have any doxygen comments: https://github.com/apache/incubator-pagespeed-mod/blob/master/devel/doxify.sh .

[jmarantz]

Most importantly, I'm very happy you have taken this on. Thank you! Happy to review as you go forward.

[markmandel]

There's no envyproxy/envoy PR for this yet, right?

No yet no - without Doxygen and graphviz in the build image, there was no way (I could find) to make a PR that would actually pass. So I started with the build image.

Also, given how large a document set Doxygen generates at 800MB+ (I got that down from ~2GB through converting images to SVG, etc), I wasn't sure if that was something we wanted to take live on every push.

I did not find the output as useful as I was hoping, but you may have better luck

As a new person to the Envoy codebase, I found it super useful. While you are right, there are missing docs in a bunch of places, even being able to see hierarchies, pure virtual methods, and where things get overwritten is very useful to work out how the code base works. Also my IDE doesn't necessarily code-complete in the most reliable way, so having something like Doxygen makes that easier as well to work out what is where.

It would be really great though to get this all done, particularly if current 'master' can have a published doc-link, kept up-to-date automatically.

So I'm fairly sure I have this (or close to this at least) already on my envoy fork. I can submit the PR once the build image PR is in. I even added a link on the "Extending Envoy" page to the Doxygen API - I figured that would be a good place to link it. Once it's live, a link on the DEVELOPER.md page would probably also be useful.

In my opinion what's needed is a multi-PR effort where we wind up having invalid or missing Doxygen comments result in a format-test failure. But before you get there we need to have a multi-PR effort to correct the current codebase. To avoid falling behind during that process I think we'd need a check_format rule where any file you touch must be doxygen-correct and complete. Hopefully will spur crowdsourcing of this large effort.

This would be great long term for sure! I would posit that once we also have Doxygen documentation being built constantly, as a tool that people use - it gives a greater incentive to ensure that docs are written, and written correctly (and things get fixed as people go through as well) - so it made sense to me as a first step - to just get _something_ up, and then can iterate from there.

Most importantly, I'm very happy you have taken this on. Thank you! Happy to review as you go forward.

You are welcome! It's been a really useful tool in learning how things work in the Envoy landscape!

[markmandel]

Oh forgot to mention - I'll do a diff on your Doxyfile vs mine, and see if there are things I missed in mine that might be super useful :+1: thanks for sharing!

[htuch]

I'm generally of the opinion that the only way to make this scale and be sustainable is if we have precommit checks that ensure validity of Doxygen links. We could start by making them incremental, similar to what was done for clang-tidy. Landing some initial docs would be great though, even without this.

[mattklein123]

Thanks a ton @markmandel for working on this. Here are my thoughts on how to proceed:

• I agree with @htuch that unless we enforce doxygen in CI, it will get stale. I think we can do this incrementally across the code base. Personally, I would start with https://github.com/envoyproxy/envoy/tree/master/include/envoy as I think this is the biggest bang for the buck as it is the "public interface."
• Given the generated docs are so large, we are probably don't want to commit them to Git, but I think we could just store them in S3/etc. like we do for the code coverage reports?

[markmandel]

Given the generated docs are so large, we are probably don't want to commit them to Git, but I think we could just store them in S3/etc. like we do for the code coverage reports?

Right now I'm generating them in ./generated/docs/doxygen/html - so they are stored in a git ignored location.

So they can be pushed up to the same place the rest of the documentation snapshop can go.

I can submit a PR for review and discussion once https://github.com/envoyproxy/envoy-build-tools/pull/26 is approved and merged. Until then, I'm a bit blocked.

[markmandel]

Thanks for that merge! Will get a PR in early this week I expect :+1:

[antoniovicente]

Would it make sense to start by checking in a Doxygen config and instructions on how to generate docs locally?

[phlax]

@mattklein123 i can take a look at this if you want

[mattklein123]

IMO this is not super urgent but would certainly be nice to have. I know that @jmarantz looked at this at some point.

[markmandel]

I started this, but ended up moving on to other things -- more than happy to have someone take this over :+1:

[phlax]

i was thinking it could be a nice addition to the dev docs (#13745) but yep, got the point about not being high prio

[phlax]

@markmandel ill look over the work you did already when i get chance