Skip to content

Conversation

@maru-ava
Copy link
Contributor

@maru-ava maru-ava commented Feb 2, 2025

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 via the tmpnet e2e flag --start-collectors
  • enables start and stop of collectors via tmpnetctl

How this was tested

Need to be documented in RELEASES.md?

N/A

TODO

@maru-ava maru-ava added testing This primarily focuses on testing ci This focuses on changes to the CI process labels Feb 2, 2025
@maru-ava maru-ava self-assigned this Feb 2, 2025
@maru-ava maru-ava force-pushed the tmpnet-managed-collectors branch 2 times, most recently from 59e5357 to 15c496b Compare February 2, 2025 04:57
Comment on lines 182 to 184
// 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

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've added a comment detailing why I took this approach, PTAL.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

// 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.

Copy link
Contributor Author

@maru-ava maru-ava Feb 24, 2025

Choose a reason for hiding this comment

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

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 outlive its parent
  • child process configured with stderr and stdout set to a file opened by the parent

Base automatically changed from nix to master February 17, 2025 16:56
@maru-ava maru-ava force-pushed the tmpnet-managed-collectors branch from 15c496b to ccd3382 Compare February 22, 2025 08:20
@maru-ava maru-ava changed the base branch from master to tmpnet-simplify-monitoring-action February 22, 2025 08:20
@maru-ava maru-ava force-pushed the tmpnet-simplify-monitoring-action branch from 757b07a to 6060ccd Compare February 22, 2025 10:28
@maru-ava maru-ava force-pushed the tmpnet-managed-collectors branch from a8099ea to 34a8689 Compare February 22, 2025 10:50
@maru-ava maru-ava force-pushed the tmpnet-simplify-monitoring-action branch from 6060ccd to d857661 Compare February 22, 2025 10:51
@maru-ava 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 maru-ava force-pushed the tmpnet-managed-collectors branch from 06869f7 to e801d02 Compare February 27, 2025 13:39
Base automatically changed from tmpnet-simplify-monitoring-action to master March 1, 2025 21:55
@maru-ava maru-ava force-pushed the tmpnet-managed-collectors branch from e801d02 to 284d898 Compare March 3, 2025 14:01
@maru-ava
Copy link
Contributor Author

maru-ava commented Mar 3, 2025

Rebased

@maru-ava maru-ava force-pushed the tmpnet-managed-collectors branch from 456b45d to a9e3433 Compare March 4, 2025 14:38
@aaronbuchwald aaronbuchwald added this pull request to the merge queue Mar 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Mar 4, 2025
maru-ava and others added 12 commits March 4, 2025 18:47
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
Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
Signed-off-by: maru <maru.newby@avalabs.org>
@maru-ava maru-ava force-pushed the tmpnet-managed-collectors branch from a9e3433 to 9c408ff Compare March 4, 2025 17:53
@StephenButtolph StephenButtolph added this pull request to the merge queue Mar 4, 2025
Merged via the queue into master with commit 7b6deac Mar 4, 2025
22 checks passed
@StephenButtolph StephenButtolph deleted the tmpnet-managed-collectors branch March 4, 2025 18:24
@github-project-automation github-project-automation bot moved this from In Review 👀 to Done ✅ in avalanchego Mar 4, 2025
cam-schultz pushed a commit that referenced this pull request Mar 24, 2025
)

Signed-off-by: maru <maru.newby@avalabs.org>
Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci This focuses on changes to the CI process testing This primarily focuses on testing

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants