feat: Auto-generate documentation for jaeger-v2 configuration structs via AST #6776
Conversation
Signed-off-by: AnmolxSingh <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6776 +/- ##
==========================================
- Coverage 96.03% 96.02% -0.02%
==========================================
Files 363 364 +1
Lines 20677 20690 +13
==========================================
+ Hits 19857 19867 +10
- Misses 626 628 +2
- Partials 194 195 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
AnmolxSingh
force-pushed
the
ast
branch
2 times, most recently
from
107268e to
e5c469a
Compare
Signed-off-by: AnmolxSingh <[email protected]>
cmd/config-doc-gen/main.go
Outdated
| } | ||
|
|
||
| // Iterate over the struct fields | ||
| for _, field := range structType.Fields.List { |
There was a problem hiding this comment.
The current loop iterates over field.Names without handling fields that are embedded (anonymous fields). Adding logic to process embedded fields could improve the robustness of the documentation generation.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Anmol <[email protected]>
cmd/config-doc-gen/main.go
Outdated
| continue | ||
| } | ||
|
|
||
| structDoc := StructDoc{ |
There was a problem hiding this comment.
not every struct you encounter is a config
There was a problem hiding this comment.
are config structs only in config.go file?
There was a problem hiding this comment.
not guaranteed. Top level configs usually are, but the nested subconfigs they contain could be anywhere.
There was a problem hiding this comment.
Do we use any specific convention to identify config structs?
There was a problem hiding this comment.
sadly, there is no required interface that configs implement. The factories typically have this function
func createDefaultConfig() component.Config {
return &Config{}
}
so you could use it as an indication that Config struct is really an interesting config.
There was a problem hiding this comment.
For this i am planning to use ast.Inspect two times one for filtering the config structs returned by above function and second for printing them
Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: Anmol <[email protected]>
Signed-off-by: AnmolxSingh <[email protected]>
| // Iterate over each target directory | ||
| for _, dir := range targetDirs { | ||
| fmt.Printf("Parsing directory: %s\n", dir) | ||
| err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { |
There was a problem hiding this comment.
you are using file system access. Doesn't Go AST have other means of traversal? We have some components imported directly from OTEL, with their own configs, and they will not be easily accessible via local file system (they would be somewhere in Go cache).
There was a problem hiding this comment.
that would require more dependencies to be added to go.mod
There was a problem hiding this comment.
golang.org/x/tools/go/packages would be added to traverse AST based on package
Signed-off-by: AnmolxSingh <[email protected]>
AnmolxSingh
changed the title
Which problem is this PR solving?
Description of the changes
Rough outline of the milestones: (Copied from Issue)
comments)
How was this change tested?
struct_docs.json
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test