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

rpc: add regex pattern filtering to warnet logs rpc #576

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

Conversation

ismaelsadeeq
Copy link
Contributor

@ismaelsadeeq ismaelsadeeq commented Sep 10, 2024

This PR updated the warnet logs rpc to now have an optional grep option which will
take a string value and filter tank logs based on regex matching with the passed string.

This can be tested by

warnet log tank-001 --grep "pattern"

Usage: warnet logs [OPTIONS] [POD_NAME]

  Show the logs of a Kubernetes pod, optionally filtering by a grep pattern.

Options:
  -f, --follow     Follow logs in real-time
  -g, --grep TEXT  Pattern to grep for in logs
  --help           Show this message and exit.

@willcl-ark
Copy link
Contributor

Thanks! This is a really nice idea and looks like it works well already.

I think the implementation could be even more awesome if it worked a little more like the logs command:

@click.command()
@click.argument("pod_name", type=str, default="")
@click.option("--follow", "-f", is_flag=True, default=False, help="Follow logs")
def logs(pod_name: str, follow: bool):
"""Show the logs of a pod"""
follow_flag = "--follow" if follow else ""
namespace = get_default_namespace()
if pod_name:
try:
command = f"kubectl logs pod/{pod_name} -n {namespace} {follow_flag}"
stream_command(command)
return
except Exception as e:
print(f"Could not find the pod {pod_name}: {e}")
try:
pods = run_command(f"kubectl get pods -n {namespace} -o json")
pods = json.loads(pods)
pod_list = [item["metadata"]["name"] for item in pods["items"]]
except Exception as e:
print(f"Could not fetch any pods in namespace {namespace}: {e}")
return
if not pod_list:
print(f"Could not fetch any pods in namespace {namespace}")
return
q = [
inquirer.List(
name="pod",
message="Please choose a pod",
choices=pod_list,
)
]
selected = inquirer.prompt(q, theme=GreenPassion())
if selected:
pod_name = selected["pod"]
try:
command = f"kubectl logs pod/{pod_name} -n {namespace} {follow_flag}"
stream_command(command)
except Exception as e:
print(f"Please consider waiting for the pod to become available. Encountered: {e}")
else:
pass # cancelled by user

This gives you a "selector" to choose a running tank from. If you did fancy taking this approach, perhaps --tank could be a flag (or could rename --select or something) which would start the tank selector menu. The selected tank could then be the only tank in the tanks variable and no modification to get_mission would be needed (I think).

@josibake
Copy link
Collaborator

Hm, currently we have warnet logs , which allows you to get the logs from a scenario container or a bitcoin tank. I'd prefer if we used that exclusively for accessing logs, instead of having one way to get logs from a commander, one way to get logs from a bitcoin node, and in the future when we add LN back in, a different way to get logs from a LN node.

@ismaelsadeeq any interest in adding a --grep argument to the warnet logs command? I think this would solve your use case and avoid you needing to reimplement the selector logic here.

We might also want to remove grep-logs entirely in the future in favor of warnet logs --all --grep "my pattern" which would stream logs from all containers and allow the user to grep for a pattern

@ismaelsadeeq
Copy link
Contributor Author

Thank you @willcl-ark @josibake for your reviews.

I will get back to this soon.

@ismaelsadeeq ismaelsadeeq marked this pull request as draft September 11, 2024 17:14
@bdp-DrahtBot bdp-DrahtBot mentioned this pull request Sep 19, 2024
@bdp-DrahtBot
Copy link
Collaborator

bdp-DrahtBot commented Sep 19, 2024

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #614 (swap out kubectl by mplsgrant)
  • #612 (Add --debug option to run scenarios for faster development by pinheadmz)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@ismaelsadeeq ismaelsadeeq force-pushed the 09-2024-warnet-filter-logs-by-tank branch from 3306f28 to 9d3e129 Compare September 27, 2024 12:32
@ismaelsadeeq ismaelsadeeq changed the title enable warnet bitcoin grep-logs rpc to filter logs by tank name rpc: add regex pattern filtering to warnet logs rpc Sep 27, 2024
@ismaelsadeeq ismaelsadeeq force-pushed the 09-2024-warnet-filter-logs-by-tank branch 2 times, most recently from 8c98502 to 8f4ddd3 Compare September 27, 2024 12:40
@ismaelsadeeq ismaelsadeeq force-pushed the 09-2024-warnet-filter-logs-by-tank branch from 8f4ddd3 to bef941d Compare September 27, 2024 12:43
@ismaelsadeeq ismaelsadeeq marked this pull request as ready for review September 27, 2024 12:45
Copy link
Contributor Author

@ismaelsadeeq ismaelsadeeq left a comment

Choose a reason for hiding this comment

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

I have not encountered logs cli prior to your comment @josibake.
What I would like to do can even simply be done using logs rpc by piping the result of logs with grep command.

But I believe the approach you mentioned is cleaner and faster hence I updated the PR to your suggestion.

I force-pushed to implement it, and updated the PR description and title to match what the C.L does.

I think adding --all option to logs rpc, and getting rid of bitcoin grep-logs can be added in a follow-up after this.

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.

4 participants