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

docs(external): fix enrichment_table and secret docs #22319

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

titaneric
Copy link
Contributor

Summary

I try to fix the issue mentioned in #22234, and I also find the similar change is needed in enrichment_table when I dive deep into the document.

I find the secret is not a simple enum, and it's more like a classic Rust enum which may have different optional fields.
Listened the hint by @pront, I find the kind field is optional field when type is counter in log_to_metrics transform, which is similar to our case.

The log_to_metrics docs insert a optional relevant_when field to let the reader knows that this is a optional field in certain case. I try to insert the same notice to both enrichment_table and secret.

Please note that there are some big change in the PR. At first, I put all of the type descriptions above, and try to merge them into concise one. Secondly, I would merge the optional field description as well when there are conflicted name (path) for different type.

Change Type

  • Bug fix
  • New feature
  • Non-functional (chore, refactoring, docs)
  • Performance

Is this a breaking change?

  • Yes
  • No

How did you test this PR?

make run-production-site-locally

Does this PR include user facing changes?

  • Yes. Please add a changelog fragment based on our guidelines.
  • No. A maintainer will apply the "no-changelog" label to this PR.

Checklist

  • Please read our Vector contributor resources.
    • make check-all is a good command to run locally. This check is
      defined here. Some of these
      checks might not be relevant to your PR. For Rust changes, at the very least you should run:
      • cargo fmt --all
      • cargo clippy --workspace --all-targets -- -D warnings
      • cargo nextest run --workspace (alternatively, you can run cargo test --all)
  • If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
    run dd-rust-license-tool write to regenerate the license inventory and commit the changes (if any). More details here.

References

Closes: #22234

It looks strange in this PR, but I think a picture worths thousands of world.

Secret description:
image

Secret type:
image

Secret optional fields:
image

@titaneric titaneric requested review from a team as code owners January 29, 2025 17:39
@github-actions github-actions bot added the domain: external docs Anything related to Vector's external, public documentation label Jan 29, 2025
@pront
Copy link
Member

pront commented Jan 29, 2025

Thank you @titaneric, I will take a look.

AFAIK, this was edited manually right? Please also see this comment thread: #21348 (comment)

@titaneric
Copy link
Contributor Author

titaneric commented Jan 29, 2025

I have read the issue thread. Yes, I edit this document manually since I am not sure how to generate the CUE document from Rust code. I could dig deeper and try to help if some hint is given.

@pront pront added the no-changelog Changes in this PR do not need user-facing explanations in the release changelog label Jan 29, 2025
Copy link
Member

@pront pront left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @titaneric, this is a great improvement. Hopefully we will get to the bottom of this #22319 (comment) as well in the future.

@pront
Copy link
Member

pront commented Jan 29, 2025

Let's also wait for @vectordotdev/documentation to take a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: external docs Anything related to Vector's external, public documentation no-changelog Changes in this PR do not need user-facing explanations in the release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing documentation for secrets
2 participants