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

Add tests that verify existing of component documentation #1025

Closed
tigrannajaryan opened this issue May 26, 2020 · 8 comments
Closed

Add tests that verify existing of component documentation #1025

tigrannajaryan opened this issue May 26, 2020 · 8 comments
Assignees
Milestone

Comments

@tigrannajaryan
Copy link
Member

We want to have documentation tests that verify that all components have a section in the corresponding README.md.

@tigrannajaryan tigrannajaryan added feature request help wanted Good issue for contributors to OpenTelemetry Service to pick up and removed feature request labels May 26, 2020
@tigrannajaryan
Copy link
Member Author

@asuresh4 will work on this (I cannot assign yet, since you have no PRs in this repo).

@tigrannajaryan tigrannajaryan removed the help wanted Good issue for contributors to OpenTelemetry Service to pick up label May 27, 2020
@asuresh4
Copy link
Member

@tigrannajaryan - I assume we are looking to add the same verification to the contrib repo as well?

Today all the documentation for components of a specific seem to be in a single README. For example, documentation related to all processors are in processors/README.md. As more components are added, seems to me like this could get cumbersome. I was wondering if it made sense to move the documentation to each component, i.e. have the README files within the component package (as we currently do in the contrib repo). What do you think?

Also, I was thinking about generating the README from a yaml where the yaml would provide some structure to the doc along with the contents required to generate the README. This way a standardized format/layout can be enforced across the READMEs.

For example,

component_name: "Span Processor"
component_type: processor/span
release: ga
doc:
    |
     Describe what the processor does and give examples.

would result in a README like

# Span Processor

release: **GA**

type: `span`

Describe what the processor does and give examples.

What do you think @tigrannajaryan ?

@jrcamp
Copy link
Contributor

jrcamp commented May 29, 2020

If we decide to put other metadata in yaml I think we can fold in #985 and have just one file. The SFX agent does this with a single metadata.yaml file. I think it is the best long term approach. If we want to still have README.md in the repo (which is useful when browsing source) it does mean we will need to have a pre-commit step to generate them. We can use Git hooks and tools like https://pre-commit.com/ to make this automatic for developers.

Having all the README.mds combined into a top-level processors/README.md is useful as well since you don't have to open a individual README but I assume we could generate those as well.

I'd drop component_ from name and type though.

@asuresh4
Copy link
Member

asuresh4 commented Jun 1, 2020

I synced with Tigran on this offline and he seemed to partly agree on generating the docs from something like yaml in the long term but details need to be flushed out. However, he mentioned for the purposes of this issue it would suffice to verify existence of docs for all components. Based on the discussion, for now this would entail splitting out the component docs into separate READMEs and automating the verification of those READMEs.

But yes, I think we should combine the metric documentation along with the component docs in the long term.

@tigrannajaryan tigrannajaryan added this to the Beta 0.4 milestone Jun 3, 2020
@asuresh4
Copy link
Member

asuresh4 commented Jun 4, 2020

Once the READMEs are moved into the directory corresponding to the component, the READMEs could potentially end up in different locations. For example, the README for attributesprocessor would be placed in processor/attributesprocessor (this covers most of the current cases). However, there are cases where this is not true. For example, README for tailsamplingprocessor will live in processor/samplingprocessor/tailsamplingprocessor. This means the verification script would somehow need to know where READMEs need to exist.

I wrote a go test that derives a list of enabled components by leveraging imports in service/defaultcomponents/defaults.go file and the then it checks for existence of a README in those locations.

Here's the test: ddbb166

What do you think @tigrannajaryan @jrcamp ?

@asuresh4
Copy link
Member

asuresh4 commented Jun 4, 2020

If we are ok with this approach I believe it could also be extended to the contrib repository.

@tigrannajaryan
Copy link
Member Author

@asuresh4 the approach looks good to me. That's a nice way of finding the location of README.

@jrcamp
Copy link
Contributor

jrcamp commented Jun 4, 2020

Generally lgtm, will make some comments when you open PR on it.

tigrannajaryan pushed a commit that referenced this issue Jun 12, 2020
Add a simple go test to ensure existence of documentation for enabled components.

**Link to tracking Issue:** #1025
wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this issue Jul 13, 2020
Add a simple go test to ensure existence of documentation for enabled components.

**Link to tracking Issue:** open-telemetry#1025
swiatekm pushed a commit to swiatekm/opentelemetry-collector that referenced this issue Oct 9, 2024
…emetry#1025)

* add helper to set kind in HPA

* update autoscaling comment in values

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

No branches or pull requests

3 participants