Skip to content

Update windows exporter dependency to v0.31.1 #4037

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

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

dehaansa
Copy link
Contributor

@dehaansa dehaansa commented Jul 22, 2025

PR Description

Updating the windows exporter dependency, again. A few more breaking changes, a few upgrades, and a few panic fixes.

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

Copy link
Contributor

github-actions bot commented Jul 22, 2025

@@ -1,6 +1,6 @@
prometheus.remote_write "metrics_default" {
endpoint {
name = "default-b174ee"
name = "default-04b53d"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is broken on main, probably from prom3.4 update.

@@ -18,7 +18,7 @@ prometheus.scrape "metrics_name_jobName" {

prometheus.remote_write "metrics_name" {
endpoint {
name = "name-b174ee"
name = "name-04b53d"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is broken on main, probably from prom3.4 update.

@dehaansa dehaansa marked this pull request as ready for review July 27, 2025 02:51
@dehaansa dehaansa requested review from clayton-cornell and a team as code owners July 27, 2025 02:51
| `exclude` | `string` | Regular expression of processes to exclude. | `"^$"` | no |
| `include` | `string` | Regular expression of processes to include. | `"^.+$"` | no |

The `counter_version` may be `0`, `1`, or `2`. A value of `0` uses the latest available (currently V2), the other values specify which version of the process collector to utilize.
Copy link
Contributor

Choose a reason for hiding this comment

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

In the docs it's not clear what the difference in the versions is. Also, do you think it makes sense to default to v1 so that there are no breaking changes? And to open a "breaking change" Alloy GitHub issue for us to switch for v2 for Alloy v2? Is v1 going to be removed from the upstream exporter soon? If it's due to be removed soon, then we'll need to switch users to v2 sooner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't be a breaking change, just an underlying implementation change, but I'll see if I can make some more clear documentation.

Makefile Outdated
@@ -125,7 +125,8 @@ GO_LDFLAGS := -X $(VPREFIX).Branch=$(GIT_BRANCH) \
-X $(VPREFIXSYNTAX).Version=$(VERSION) \
-X $(VPREFIX).Revision=$(GIT_REVISION) \
-X $(VPREFIX).BuildUser=$(shell whoami)@$(shell hostname) \
-X $(VPREFIX).BuildDate=$(shell date -u +"%Y-%m-%dT%H:%M:%SZ")
-X $(VPREFIX).BuildDate=$(shell date -u +"%Y-%m-%dT%H:%M:%SZ") \
-ld64
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do? Why is it necessary?

@ptodev ptodev self-assigned this Jul 28, 2025
The `textfile` collector is currently configured with the `text_file` block.
To be consistent with the `textfile` collector name, the `text_file` block will be deprecated in a future release and replaced with a `textfile` block.
For backwards compatibility, the `textfile` collector can also be configured with the undocumented `text_file` block.
It is identical to the `textfile` block, and will be removed in a future release.
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 wondering if we should leave the text_file block in the docs, but marked as deprecated until it's actually removed.

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 open to it, I chose this approach as it pushes users towards the corrected block name but they can still find the block if they're searching through the docs for text_file.

dehaansa and others added 2 commits July 28, 2025 11:22
@@ -69,11 +69,11 @@ You can use the following blocks with `prometheus.exporter.windows`:
| [`smb`][smb] | Configures the `smb` collector. | no |
| [`smtp`][smtp] | Configures the `smtp` collector. | no |
| [`tcp`][tcp] | Configures the `tcp` collector. | no |
| [`text_file`][text_file] | Configures the `textfile` collector. | no |
| [`textfile`][textfile] | Configures the `textfile` collector. | no |
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
| [`textfile`][textfile] | Configures the `textfile` collector. | no |
| [`text_file`][text_file] | (Deprecated: use `textfile` instead) Configures the `textfile` collector. | no |
| [`textfile`][textfile] | Configures the `textfile` collector. | no |

We could document like this (as used elsewhere in the component docs - eg: in otelcol.connector.spanmetrics). We are a little inconsistent with how we document deprecated args and blocks. I just picked the style that provides the most useful information.

Comment on lines +75 to +76
For backwards compatibility, the `textfile` collector can also be configured with the undocumented `text_file` block.
It is identical to the `textfile` block, and will be removed in a future release.
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
For backwards compatibility, the `textfile` collector can also be configured with the undocumented `text_file` block.
It is identical to the `textfile` block, and will be removed in a future release.
{{< admonition type="caution" >}}
Starting with v1.11, the `text_file ` block is deprecated.
It will be removed in a future release.
Use the `textfile` block to configure the `textfile` collector.
{{< /admonition >}}

Something like this to follow the advice here: https://grafana.com/docs/writers-toolkit/write/deprecate-remove/ (assuming the deprecation happens in 1.11?

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

Successfully merging this pull request may close these issues.

3 participants