[tmpnet] Deploy collectors with golang to simplify cross-repo use #3692

maru-ava · 2025-02-02T04:50:14Z

Why this should be merged

Previously, prometheus and promtail for collecting from local tmpnet networks were installed and launched by bash scripts. Migrating installation to nix and launch to golang simplifies reuse from subnet-evm and hypersdk. No more having to copy and maintain copies of the scripts in multiple repos.

How this works

migrates scripts launching prometheus and promtail to golang
configures collector start by via the tmpnet e2e flag --start-collectors
enables start and stop of collectors via tmpnetctl

How this was tested

CI for regression
Manually checked that metrics and logs were collected from local and CI networks with the chnages
- Automatic checks implemented in [tmpnet] Add check for collection of logs and metrics to custom github action #3740

Need to be documented in RELEASES.md?

N/A

TODO

Merge [testing] Replace script-based tool installation with nix #3691
Refactor for review
Merge [ci] Simplify tmpnet monitoring action #3736

aaronbuchwald · 2025-02-06T22:56:33Z

tests/fixture/tmpnet/start_collectors.go

+	// TODO(marun) Figure out a way to redirect stdout and stderr of a detached child process without a bash shell
+	cmd := exec.Command("bash", "-c", fullCmd)
+	configureDetachedProcess(cmd) // Ensure the child process will outlive its parent


I believe you can create the command and set Stdout and Stderr to nil directly (according to the docs)

Please give it a try and get back to me, because my attempts to create a child process that outlives its parent and configured with stdout and stderr redirected to a file created and opened by that parent were entirely unsuccessful.

I've added a comment detailing why I took this approach, PTAL.

I may be wrong here, just remembered reading the documentation around this, so thought I'd provide the suggestion. May not behave as I expected from reading the docs (either due to a misread or incorrect docs).

Re-reading your original comment, what is the intended result of setting Stdout and Stderr to nil? I need to send the output to a file, otherwise in CI I won't be able to collect the logs for troubleshooting (from the same tmpnet dir artifacts that contain the networks).

I think that I thought you were trying to ignore the output here:

avalanchego/tests/fixture/tmpnet/start_collectors.go

Line 182 in 15c496b

// TODO(marun) Figure out a way to redirect stdout and stderr of a detached child process without a bash shell

, but you actually wanted to write it to the specified file. I think you could still do this by setting Stdout and Stderr to the corresponding file and avoid constructing a bash command.

I attempted to do just that, but the following combination did not result in a log file that the child was writing to:

child process that will outlife its parent

child process configured with stderr and stdout set to a file opened by the parent

maru-ava · 2025-02-24T14:07:11Z

tests/fixture/tmpnet/README.md

+    loki_password: ${{ secrets.LOKI_PASSWORD || '' }}
+```
+
+The action assumes a nix flake file in the repo root that enables


I guess the multiple apostrophes (') above are causing this formatting issue? I'm not inclined to try to fix this, since the above is verbatim how the action is expected to be used.

aaronbuchwald · 2025-02-24T18:45:49Z

tests/fixture/e2e/flags.go

 func (v *FlagVars) NetworkShutdownDelay() time.Duration {
-	if v.delayNetworkShutdown {
+	if v.startCollectors {


Why is startCollectors replacing delayNetworkShutdown and being used in NetworkShutdownDelay() ?

Ah ok, the network shutdown delay was added to account for a final network scrape, so we are removing the old flag and checking startCollectors to decide whether we need a non-zero network shutdown delay.

aaronbuchwald · 2025-02-24T18:49:41Z

.github/actions/run-monitored-tmpnet-cmd/action.yml

-      env:
-        LOKI_USERNAME: ${{ inputs.loki_username }}
-        LOKI_PASSWORD: ${{ inputs.loki_password }}
+    # TODO(marun) Stop emitting this annotation so that any annotation that appears can be assumed to be actionable


What annotation is this referring to?

The script that this step calls creates a github actions 'annotation' that appears on the summary page of the run. But I'm thinking it does more harm than good since the only other annotations are typically warnings from actions warning of deprecations that we need to notice and respond to.

aaronbuchwald · 2025-02-24T19:41:47Z

tests/fixture/tmpnet/start_collectors.go

+	// TODO(marun) Figure out a way to redirect stdout and stderr of a detached child process without a bash shell
+	cmd := exec.Command("bash", "-c", fullCmd)
+	configureDetachedProcess(cmd) // Ensure the child process will outlive its parent


I think that I thought you were trying to ignore the output here:

avalanchego/tests/fixture/tmpnet/start_collectors.go

Line 182 in 15c496b

// TODO(marun) Figure out a way to redirect stdout and stderr of a detached child process without a bash shell

, but you actually wanted to write it to the specified file. I think you could still do this by setting Stdout and Stderr to the corresponding file and avoid constructing a bash command.

Previously, prometheus and promtail were installed and launched by with bash scripts. Migrating installation to nix and launch to golang enables directly sharing the functionality with subnet-evm and hypersdk. No more having to copy and maintain copies of the scripts in multiple repos.

- ensure started/stopped -> start/stop collectors - use the same helpers between collector start and node process

maru-ava added testing This primarily focuses on testing ci This focuses on changes to the CI process labels Feb 2, 2025

maru-ava self-assigned this Feb 2, 2025

maru-ava force-pushed the tmpnet-managed-collectors branch 2 times, most recently from 59e5357 to 15c496b Compare February 2, 2025 04:57

This was referenced Feb 2, 2025

[testing] Simplify log and metric collection ava-labs/subnet-evm#1440

Draft

Enable collection of metrics and logs from tmpnet networks in CI ava-labs/hypersdk#1909

Open

aaronbuchwald reviewed Feb 6, 2025

View reviewed changes

maru-ava force-pushed the nix branch from 827d7ea to fecaf24 Compare February 17, 2025 12:59

Base automatically changed from nix to master February 17, 2025 16:56

maru-ava force-pushed the tmpnet-managed-collectors branch from 15c496b to ccd3382 Compare February 22, 2025 08:20

maru-ava changed the base branch from master to tmpnet-simplify-monitoring-action February 22, 2025 08:20

maru-ava force-pushed the tmpnet-simplify-monitoring-action branch from 757b07a to 6060ccd Compare February 22, 2025 10:28

maru-ava force-pushed the tmpnet-managed-collectors branch from a8099ea to 34a8689 Compare February 22, 2025 10:50

maru-ava force-pushed the tmpnet-simplify-monitoring-action branch from 6060ccd to d857661 Compare February 22, 2025 10:51

maru-ava force-pushed the tmpnet-managed-collectors branch 15 times, most recently from 01300b7 to 266cdb9 Compare February 22, 2025 19:48

maru-ava force-pushed the tmpnet-simplify-monitoring-action branch from 0c9fe11 to b1e800e Compare February 22, 2025 19:50

maru-ava force-pushed the tmpnet-managed-collectors branch from 266cdb9 to 194a2d3 Compare February 22, 2025 19:52

maru-ava marked this pull request as ready for review February 22, 2025 19:56

maru-ava requested a review from StephenButtolph as a code owner February 22, 2025 19:56

maru-ava force-pushed the tmpnet-managed-collectors branch 3 times, most recently from 5547800 to 0790dd1 Compare February 24, 2025 13:07

maru-ava force-pushed the tmpnet-simplify-monitoring-action branch from b1e800e to 260259e Compare February 24, 2025 13:08

maru-ava force-pushed the tmpnet-managed-collectors branch 5 times, most recently from 4daa019 to 31be45a Compare February 24, 2025 13:35

maru-ava mentioned this pull request Feb 24, 2025

[tmpnet] Add check for collection of logs and metrics to custom github action #3740

Open

4 tasks

maru-ava commented Feb 24, 2025

View reviewed changes

maru-ava force-pushed the tmpnet-managed-collectors branch from 91b64a7 to 4dce5a1 Compare February 24, 2025 15:34

aaronbuchwald reviewed Feb 24, 2025

View reviewed changes

maru-ava force-pushed the tmpnet-simplify-monitoring-action branch from 260259e to af785f9 Compare February 24, 2025 21:32

maru-ava added 8 commits February 24, 2025 22:33

fixup: Refactor for review

bd6e9e2

fixup: Cleanup for consistency

970c8bf

- ensure started/stopped -> start/stop collectors - use the same helpers between collector start and node process

fixup: More cleanup

43f269e

fixup: Add README for action

d68b12f

fixup: Fix args

6f20e6f

fixup: Remove apostrophe to avoid github's shite formatting

9ebebef

fixup: Move promtail readiness check to after nodes have started

1a5222c

maru-ava force-pushed the tmpnet-managed-collectors branch from 4dce5a1 to 1a5222c Compare February 24, 2025 21:33

maru-ava added 2 commits February 24, 2025 23:27

fixup: Inline step uploading tmpnet network data

0faf3d9

fixup: Update README to mention the need to vendor tmpnet

06869f7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tmpnet] Deploy collectors with golang to simplify cross-repo use #3692

[tmpnet] Deploy collectors with golang to simplify cross-repo use #3692

maru-ava commented Feb 2, 2025 •

edited

Loading

aaronbuchwald Feb 6, 2025

maru-ava Feb 7, 2025

maru-ava Feb 22, 2025

aaronbuchwald Feb 24, 2025

maru-ava Feb 24, 2025

aaronbuchwald Feb 24, 2025

maru-ava Feb 24, 2025

maru-ava Feb 24, 2025

aaronbuchwald Feb 24, 2025

aaronbuchwald Feb 24, 2025

aaronbuchwald Feb 24, 2025

maru-ava Feb 24, 2025

aaronbuchwald Feb 24, 2025

[tmpnet] Deploy collectors with golang to simplify cross-repo use #3692

Are you sure you want to change the base?

[tmpnet] Deploy collectors with golang to simplify cross-repo use #3692

Conversation

maru-ava commented Feb 2, 2025 • edited Loading

Why this should be merged

How this works

How this was tested

Need to be documented in RELEASES.md?

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maru-ava commented Feb 2, 2025 •

edited

Loading