Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 114 additions & 2 deletions src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,29 @@ impl<'a> AuthenticatedApi<'a> {
Ok(rv)
}

/// Fetch organization events from the specified dataset
pub fn fetch_organization_events(
Comment thread
vgrozdanic marked this conversation as resolved.
&self,
org: &str,
options: &FetchEventsOptions,
) -> ApiResult<Vec<LogEntry>> {
let params = options.to_query_params();
let url = format!(
"/organizations/{}/events/?{}",
PathArg(org),
params.join("&")
);

let resp = self.get(&url)?;

if resp.status() == 404 {
return Err(ApiErrorKind::OrganizationNotFound.into());
}

let logs_response: LogsResponse = resp.convert()?;
Ok(logs_response.data)
}

/// List all issues associated with an organization and a project
pub fn list_organization_project_issues(
&self,
Expand Down Expand Up @@ -1390,6 +1413,78 @@ impl<'a> AuthenticatedApi<'a> {
}
}

/// Available datasets for fetching organization events
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Dataset {
/// Our logs dataset
OurLogs,
Comment thread
shellmayr marked this conversation as resolved.
Outdated
}

impl Dataset {
/// Returns the string representation of the dataset
fn as_str(&self) -> &'static str {
match self {
Dataset::OurLogs => "ourlogs",
Comment thread
shellmayr marked this conversation as resolved.
Outdated
}
Comment thread
shellmayr marked this conversation as resolved.
}
}
Comment thread
vgrozdanic marked this conversation as resolved.

impl fmt::Display for Dataset {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.as_str())
}
}

/// Options for fetching organization events
pub struct FetchEventsOptions<'a> {
/// Dataset to fetch events from
pub dataset: Dataset,
/// Fields to include in the response
pub fields: &'a [&'a str],
/// Project ID to filter events by
pub project_id: Option<&'a str>,
Comment thread
szokeasaurusrex marked this conversation as resolved.
Outdated
/// Cursor for pagination
pub cursor: Option<&'a str>,
Comment thread
szokeasaurusrex marked this conversation as resolved.
/// Query string to filter events
pub query: Option<&'a str>,
/// Number of events per page (default: 100)
pub per_page: Option<usize>,
/// Time period for stats (default: "1h")
pub stats_period: Option<&'a str>,
/// Sort order (default: "-timestamp")
pub sort: Option<&'a str>,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For now, all of these fields appear to always be being set to a Some variant, so the default value never ends up being used.

Therefore, I would like to see the struct fields be made non-optional, so we can also get rid of the defaults in the code, and make the API simpler. We can always later make the fields optional, as needed.

Suggested change
/// Query string to filter events
pub query: Option<&'a str>,
/// Number of events per page (default: 100)
pub per_page: Option<usize>,
/// Time period for stats (default: "1h")
pub stats_period: Option<&'a str>,
/// Sort order (default: "-timestamp")
pub sort: Option<&'a str>,
/// Query string to filter events
pub query: &'a str,
/// Number of events per page
pub per_page: usize,
/// Time period for stats
pub stats_period: &'a str,
/// Sort order
pub sort: &'a str,

Copy link
Copy Markdown
Member

@shellmayr shellmayr Aug 6, 2025

Choose a reason for hiding this comment

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

@szokeasaurusrex Calls to the endpoint don't need to have a query, stats_period, per_page or sort. What are we achieving by making them non-optional?

Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex Aug 6, 2025

Choose a reason for hiding this comment

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

I want to keep the Sentry CLI code simple. Making these optional means we have to add code paths to Sentry CLI, which for now, are not being used, because we always set the query, stats_period, per_page, and sort. Rust, however, still forces us to handle the cases of these being None (even though, in reality, they never are None).

As a result, we have code paths, which are completely unused, to handle the None cases. In the case of the query, we only provide it if it is there. For stats_period, per_page, and sort, we have default values defined, which are never used. From reading the code, however, it is not immediately clear that these default values are never used.

The problem with keeping them as Option is just that it increases complexity and it makes it harder to see where the actual values get defined. I understand that the values are optional from the server perspective, but that absolutely does not mean that we need to make them optional in the CLI's API

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@shellmayr As an alternative, I would also be okay with just hardcoding the defaults and removing these fields from the struct completely. I'm okay with whatever you think is more reasonable, just want to avoid the Option when its adding currently unnecessary complexity by handling cases which never occur.

}

impl<'a> FetchEventsOptions<'a> {
/// Generate query parameters as a vector of strings
pub fn to_query_params(&self) -> Vec<String> {
let mut params = vec![format!("dataset={}", QueryArg(self.dataset.as_str()))];

for field in self.fields {
params.push(format!("field={}", QueryArg(field)));
}

if let Some(cursor) = self.cursor {
params.push(format!("cursor={}", QueryArg(cursor)));
}

if let Some(project_id) = self.project_id {
params.push(format!("project={}", QueryArg(project_id)));
}

if let Some(query) = self.query {
params.push(format!("query={}", QueryArg(query)));
}

params.push(format!("per_page={}", self.per_page.unwrap_or(100)));
params.push(format!("statsPeriod={}", self.stats_period.unwrap_or("1h")));

params.push(format!("sort={}", self.sort.unwrap_or("-timestamp")));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Default Parameter Inclusion Bug

The FetchEventsOptions::to_query_params method incorrectly includes per_page, statsPeriod, and sort parameters in the query string with default values, even when their Option fields are None. This is due to the use of unwrap_or(), which forces their inclusion, contradicting their intended optional behavior and the API's design. In contrast, cursor and query parameters are correctly added only when explicitly provided.

Fix in Cursor Fix in Web

params
}
Comment thread
szokeasaurusrex marked this conversation as resolved.
}

impl RegionSpecificApi<'_> {
fn request(&self, method: Method, url: &str) -> ApiResult<ApiRequest> {
self.api
Expand Down Expand Up @@ -2343,7 +2438,7 @@ pub struct ProcessedEvent {
pub tags: Option<Vec<ProcessedEventTag>>,
}

#[derive(Clone, Debug, Deserialize)]
#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct ProcessedEventUser {
#[serde(skip_serializing_if = "Option::is_none")]
pub id: Option<String>,
Expand Down Expand Up @@ -2377,7 +2472,7 @@ impl fmt::Display for ProcessedEventUser {
}
}

#[derive(Clone, Debug, Deserialize)]
#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct ProcessedEventTag {
pub key: String,
pub value: String,
Expand All @@ -2401,3 +2496,20 @@ pub struct Region {
pub struct RegionResponse {
pub regions: Vec<Region>,
}

/// Response structure for logs API
#[derive(Debug, Deserialize)]
struct LogsResponse {
data: Vec<LogEntry>,
}

/// Log entry structure from the logs API
#[derive(Debug, Deserialize)]
pub struct LogEntry {
#[serde(rename = "sentry.item_id")]
pub item_id: String,
pub trace: Option<String>,
pub severity: Option<String>,
pub timestamp: String,
pub message: Option<String>,
}
2 changes: 2 additions & 0 deletions src/commands/derive_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::utils::auth_token::AuthToken;
use crate::utils::value_parsers::{auth_token_parser, kv_parser};
use clap::{command, ArgAction::SetTrue, Parser, Subcommand};

use super::logs::LogsArgs;
use super::send_metric::SendMetricArgs;

#[derive(Parser)]
Expand Down Expand Up @@ -32,5 +33,6 @@ pub(super) struct SentryCLI {

#[derive(Subcommand)]
pub(super) enum SentryCLICommand {
Logs(LogsArgs),
SendMetric(SendMetricArgs),
}
132 changes: 132 additions & 0 deletions src/commands/logs/list.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
use anyhow::Result;
use clap::Args;

use crate::api::{Api, Dataset, FetchEventsOptions};
use crate::config::Config;
use crate::utils::formatting::Table;

const MAX_ROWS_RANGE: std::ops::RangeInclusive<usize> = 1..=1000;
/// Validate that max_rows is in the allowed range
fn validate_max_rows(s: &str) -> Result<usize> {
let value = s.parse()?;
if MAX_ROWS_RANGE.contains(&value) {
Ok(value)
} else {
Err(anyhow::anyhow!(
"max-rows must be between {} and {}",
MAX_ROWS_RANGE.start(),
MAX_ROWS_RANGE.end()
))
}
}

/// Fields to fetch from the logs API
const LOG_FIELDS: &[&str] = &[
"sentry.item_id",
"trace",
"severity",
"timestamp",
"message",
];

/// Arguments for listing logs
#[derive(Args)]
pub(super) struct ListLogsArgs {
#[arg(short = 'o', long = "org")]
#[arg(help = "The organization ID or slug.")]
org: Option<String>,

#[arg(short = 'p', long = "project")]
#[arg(help = "The project ID (slug not supported).")]
project: Option<String>,

#[arg(long = "max-rows", default_value = "100")]
#[arg(value_parser = validate_max_rows)]
#[arg(help = format!("Maximum number of log entries to fetch and display (max {}).", MAX_ROWS_RANGE.end()))]
max_rows: usize,

#[arg(long = "query", default_value = "")]
#[arg(help = "Query to filter logs. Example: \"level:error\"")]
query: String,
}
Comment thread
cursor[bot] marked this conversation as resolved.

pub(super) fn execute(args: ListLogsArgs) -> Result<()> {
let config = Config::current();
let (default_org, default_project) = config.get_org_and_project_defaults();

let org = args
.org
.as_ref()
.or(default_org.as_ref())
.ok_or_else(|| {
anyhow::anyhow!("No organization specified. Please specify an organization using the --org argument.")
})?;

let project = args
.project
.as_ref()
.or(default_project.as_ref())
.ok_or_else(|| {
anyhow::anyhow!("No project specified. Use --project or set a default in config.")
})?;

let api = Api::current();

let query = if args.query.is_empty() {
None
} else {
Some(args.query.as_str())
};

execute_single_fetch(&api, org, project, query, LOG_FIELDS, &args)
}

fn execute_single_fetch(
api: &Api,
org: &str,
project: &str,
query: Option<&str>,
fields: &[&str],
args: &ListLogsArgs,
) -> Result<()> {
let options = FetchEventsOptions {
dataset: Dataset::OurLogs,
fields,
project_id: Some(project),
cursor: None,
query,
per_page: Some(args.max_rows),
stats_period: Some("90d"),
sort: Some("-timestamp"),
};

let logs = api
.authenticated()?
.fetch_organization_events(org, &options)?;

let mut table = Table::new();
table
.title_row()
.add("Item ID")
.add("Timestamp")
.add("Severity")
.add("Message")
.add("Trace");
Comment thread
vgrozdanic marked this conversation as resolved.

for log in logs.iter().take(args.max_rows) {
let row = table.add_row();
row.add(&log.item_id)
.add(&log.timestamp)
.add(log.severity.as_deref().unwrap_or(""))
.add(log.message.as_deref().unwrap_or(""))
.add(log.trace.as_deref().unwrap_or(""));
}
Comment thread
cursor[bot] marked this conversation as resolved.

if table.is_empty() {
println!("No logs found");
} else {
table.print();
}

Ok(())
}
41 changes: 41 additions & 0 deletions src/commands/logs/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
mod list;

use self::list::ListLogsArgs;
use super::derive_parser::{SentryCLI, SentryCLICommand};
use anyhow::Result;
use clap::ArgMatches;
use clap::{Args, Command, Parser as _, Subcommand};

const LIST_ABOUT: &str = "List logs from your organization";

#[derive(Args)]
pub(super) struct LogsArgs {
#[command(subcommand)]
subcommand: LogsSubcommand,
}

#[derive(Subcommand)]
#[command(about = "Manage logs in Sentry")]
#[command(long_about = "Manage and query logs in Sentry. \
This command provides access to log entries.")]
enum LogsSubcommand {
#[command(about = LIST_ABOUT)]
#[command(long_about = format!("{LIST_ABOUT}. \
Query and filter log entries from your Sentry projects. \
Supports filtering by log level and custom queries."))]
List(ListLogsArgs),
}

pub(super) fn make_command(command: Command) -> Command {
LogsSubcommand::augment_subcommands(command)
}

pub(super) fn execute(_: &ArgMatches) -> Result<()> {
let SentryCLICommand::Logs(LogsArgs { subcommand }) = SentryCLI::parse().command else {
unreachable!("expected logs subcommand");
};

match subcommand {
LogsSubcommand::List(args) => list::execute(args),
}
}
3 changes: 3 additions & 0 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ mod files;
mod info;
mod issues;
mod login;
mod logs;
mod mobile_app;
mod monitors;
mod organizations;
Expand Down Expand Up @@ -57,6 +58,7 @@ macro_rules! each_subcommand {
$mac!(info);
$mac!(issues);
$mac!(login);
$mac!(logs);
#[cfg(feature = "unstable-mobile-app")]
$mac!(mobile_app);
$mac!(monitors);
Expand Down Expand Up @@ -95,6 +97,7 @@ const UPDATE_NAGGER_CMDS: &[&str] = &[
"info",
"issues",
"login",
"logs",
"organizations",
"projects",
"releases",
Expand Down
10 changes: 4 additions & 6 deletions src/commands/send_metric/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,10 @@ pub(super) fn make_command(command: Command) -> Command {
}

pub(super) fn execute(_: &ArgMatches) -> Result<()> {
// When adding a new subcommand to the derive_parser SentryCLI, replace the line below with the following:
// let subcommand = match SentryCLI::parse().command {
// SentryCLICommand::SendMetric(SendMetricArgs { subcommand }) => subcommand,
// _ => panic!("expected send-metric subcommand"),
// };
let SentryCLICommand::SendMetric(SendMetricArgs { subcommand }) = SentryCLI::parse().command;
let subcommand = match SentryCLI::parse().command {
SentryCLICommand::SendMetric(SendMetricArgs { subcommand }) => subcommand,
_ => unreachable!("expected send-metric subcommand"),
};
Comment thread
vgrozdanic marked this conversation as resolved.

log::warn!("{DEPRECATION_MESSAGE}");

Expand Down
1 change: 1 addition & 0 deletions tests/integration/_cases/help/help-windows.trycmd
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Commands:
info Print information about the configuration and verify authentication.
issues Manage issues in Sentry.
login Authenticate with the Sentry server.
logs Manage logs in Sentry
monitors Manage cron monitors on Sentry.
organizations Manage organizations on Sentry.
projects Manage projects on Sentry.
Expand Down
1 change: 1 addition & 0 deletions tests/integration/_cases/help/help.trycmd
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Commands:
info Print information about the configuration and verify authentication.
issues Manage issues in Sentry.
login Authenticate with the Sentry server.
logs Manage logs in Sentry
monitors Manage cron monitors on Sentry.
organizations Manage organizations on Sentry.
projects Manage projects on Sentry.
Expand Down
Loading
Loading