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 docs for foreach #2447

Open
wants to merge 3 commits into
base: ptodev/dynamic-pipelines
Choose a base branch
from

Conversation

ptodev
Copy link
Contributor

@ptodev ptodev commented Jan 17, 2025

No description provided.

@ptodev ptodev requested a review from wildum January 17, 2025 18:42
Copy link
Contributor

github-actions bot commented Jan 17, 2025

docs/sources/reference/config-blocks/foreach.md Outdated Show resolved Hide resolved
docs/sources/reference/config-blocks/foreach.md Outdated Show resolved Hide resolved
docs/sources/reference/config-blocks/foreach.md Outdated Show resolved Hide resolved
docs/sources/reference/config-blocks/foreach.md Outdated Show resolved Hide resolved
docs/sources/reference/config-blocks/foreach.md Outdated Show resolved Hide resolved
Comment on lines +62 to +64
## Examples

### Run Prometheus exporters dynamically on service discovery targets
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Examples
### Run Prometheus exporters dynamically on service discovery targets
## Example
The following example shows you how to run Prometheus exporters dynamically on service discovery targets.

It's a single example and we shouldn't have headings with no text between them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sure there will be more examples in the future, so I wanted to make it a bit easier to add them by having the right structure.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can expand it then to Examples (with the s) if/when we add more examples?

`var` | `string` | Name of the variable referring to the current item in the collection. | | yes
`enable_metrics` | `bool` | Whether to expose debug metrics in Alloy's `/metrics` endpoint. | `false` | no

`collection` could contain any [type][types] such as a bool, a string, a list, or a map.
Copy link
Contributor

Choose a reason for hiding this comment

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

could contain or can contain any type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it more clear if I reword it to:

The items in the collection list can be of any type such as a bool, a string, a list, or a map.

Copy link
Contributor

@clayton-cornell clayton-cornell Jan 20, 2025

Choose a reason for hiding this comment

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

Suggested change
`collection` could contain any [type][types] such as a bool, a string, a list, or a map.
The items in the `collection` list can be of any type [type][types], such as a bool, a string, a list, or a map.

Add a comma and it should be good.

docs/sources/reference/config-blocks/foreach.md Outdated Show resolved Hide resolved
Comment on lines +68 to +70
On the other hand, `discovery.*` components such as `discovery.kubernetes` output a list of targets such as this:

{{< collapse title="Example targets output by discovery.kubernetes" >}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this should be collapsed in this context. We could expand the sentence in line 68 a bit more. or take out the collapse shortcode... or maybe collapse this whole section? It's an aside... extra info about the example further down, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I included the collapsible section is to illustrate what the contents of collection are.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, works for me :-) Leave it as-is.

docs/sources/reference/config-blocks/foreach.md Outdated Show resolved Hide resolved
Copy link
Contributor

@wildum wildum left a comment

Choose a reason for hiding this comment

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

The example is great :)

docs/sources/reference/config-blocks/foreach.md Outdated Show resolved Hide resolved
### template block

The `template` block contains the definition of Alloy components which will be ran for every item in the collection.
The contents of the block look like a normal Alloy config file, except that you can use the keyword defined in `var` to refer to the current item in the collection.
Copy link
Contributor

@wildum wildum Jan 20, 2025

Choose a reason for hiding this comment

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

We should probably specify that components inside of the template can use exports of components that are outside of the foreach block but components outside of the foreach cannot use exports from components inside of the template.

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

Successfully merging this pull request may close these issues.

3 participants