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

Add command sentry-cli debug-files bundle-jvm for bundling Java (and other JVM based languages) sources #1551

Merged
merged 32 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
ccb8b39
Add command for bundling Java sources
adinauer Mar 29, 2023
77c49a1
remove commented out code
adinauer Mar 29, 2023
05f0c7e
Allow passing in debug_id; add tests; cleanup
adinauer Apr 6, 2023
1c8034c
Fix existing tests
adinauer Apr 6, 2023
6af257e
Test help
adinauer Apr 6, 2023
2adb20d
Merge branch 'master' into feat/java-source-bundling
adinauer Apr 6, 2023
7fff72d
Fix tests after merge
adinauer Apr 6, 2023
9fc5b01
Apply fmt
adinauer Apr 6, 2023
f1a1a0e
fix clippy
adinauer Apr 6, 2023
80462d8
Add tests for windows
adinauer Apr 6, 2023
696c658
fmt
adinauer Apr 6, 2023
a883b56
add comma
adinauer Apr 6, 2023
7fea001
fix windows test
adinauer Apr 6, 2023
280c47f
revert windows test
adinauer Apr 6, 2023
4241cc4
try to fix windows test
adinauer Apr 6, 2023
3644f88
another try
adinauer Apr 6, 2023
6084679
yet another try
adinauer Apr 6, 2023
907dea9
Apply suggestions from code review
adinauer Apr 7, 2023
958c8f4
Merge windows and non windows help test
adinauer Apr 7, 2023
c749169
clippy
adinauer Apr 7, 2023
76d8a16
Update expected test output after merging suggestions (caused by added)
adinauer Apr 7, 2023
73f0436
If out dir does not exist, create it
adinauer Apr 7, 2023
629ebdb
fix tests
adinauer Apr 7, 2023
06e615b
Expect success
adinauer Apr 7, 2023
f5a2ef3
Unify windows and non windows test
adinauer Apr 7, 2023
ba0227a
Remove commented out short options
adinauer Apr 11, 2023
624a52c
PR feedback; use clap value_parser; move code; etc
adinauer Apr 11, 2023
db04bee
Merge is-file tests for windows and non windows
adinauer Apr 11, 2023
c348d99
Rename command to bundle-jvm
adinauer Apr 11, 2023
e05eacb
Rename JvmBased to Jvm
adinauer Apr 11, 2023
b83271d
fmt
adinauer Apr 11, 2023
04418d6
Forgot to add renamed dir after move
adinauer Apr 11, 2023
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
112 changes: 112 additions & 0 deletions src/commands/debug_files/create_jvm_based_bundle.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
use std::fs;
use std::path::{Path};
use anyhow::{bail, Result};
use clap::{Arg, ArgMatches, Command};
use sentry::types::DebugId;
use symbolic::debuginfo::sourcebundle::{SourceFileType};
use uuid::{Uuid};
use crate::api::Api;
use crate::config::Config;
use crate::utils::args::ArgExt;
use crate::utils::file_search::ReleaseFileSearch;
use crate::utils::file_upload::{FileUpload, SourceFile, UploadContext};
use crate::utils::fs::{path_as_url};

pub fn make_command(command: Command) -> Command {
command
.hide(true) // experimental for now
.about("Create a source bundle for the given JVM based source files (e.g. Java, Kotlin, ...)")
.org_arg()
.project_arg(false)
.arg(
Arg::new("path")
.value_name("PATH")
.required(true)
.help("The directory containing source files to bundle."),
)
.arg(
Arg::new("output")
// .short('o')
adinauer marked this conversation as resolved.
Show resolved Hide resolved
.long("output")
.value_name("PATH")
.required(true)
.help("The path to the output folder."),
)
.arg(
Arg::new("debug_id")
// .short('d')
adinauer marked this conversation as resolved.
Show resolved Hide resolved
.long("debug-id")
.value_name("UUID")
.required(true)
.help("Debug ID to use for the source bundle."),
)
}

pub fn execute(matches: &ArgMatches) -> Result<()> {
let config = Config::current();
let org = config.get_org(matches)?;
let project = config.get_project(matches).ok();
let api = Api::current();
let chunk_upload_options = api.get_chunk_upload_options(&org)?;
let context = &UploadContext {
Copy link
Contributor

Choose a reason for hiding this comment

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

@loewenheim something that we should refactor soon (whole UploadContext should have a constructor or a Default implementation, or a builder.

org: &org,
project: project.as_deref(),
release: None,
dist: None,
note: None,
wait: true,
dedupe: false,
chunk_upload_options: chunk_upload_options.as_ref(),
};
let path = Path::new(matches.get_one::<String>("path").unwrap());
let output_path = Path::new(matches.get_one::<String>("output").unwrap());
adinauer marked this conversation as resolved.
Show resolved Hide resolved
let debug_id_arg = matches.get_one::<String>("debug_id").unwrap();
let debug_id_uuid = Uuid::parse_str(debug_id_arg);
if debug_id_uuid.is_err() {
bail!("Given debug_id is invalid: {}", debug_id_arg)
}
let debug_id = DebugId::from_uuid(debug_id_uuid.unwrap());
adinauer marked this conversation as resolved.
Show resolved Hide resolved
let mut debug_id_string = debug_id.to_string();
debug_id_string.push_str(".zip");
let out = output_path.join(Path::new(&debug_id_string));
adinauer marked this conversation as resolved.
Show resolved Hide resolved

if !path.exists() {
bail!("Given path does not exist: {}", path.to_string_lossy())
adinauer marked this conversation as resolved.
Show resolved Hide resolved
}
loewenheim marked this conversation as resolved.
Show resolved Hide resolved

if path.is_dir() {
let sources = ReleaseFileSearch::new(path.to_path_buf()).collect_files()?;
let files = sources
.iter()
.map(|source| {
let local_path = source.path.strip_prefix(&source.base_path).unwrap();
let local_path_jvm_ext = local_path.with_extension("jvm");
let url = format!("{}/{}", "~", path_as_url(&local_path_jvm_ext));
adinauer marked this conversation as resolved.
Show resolved Hide resolved
(
url.to_string(),
SourceFile {
url,
path: source.path.clone(),
contents: source.contents.clone(),
ty: SourceFileType::Source,
headers: vec![],
messages: vec![],
already_uploaded: false,
},
)
})
.collect();

let copy_result = match FileUpload::new(context).files(&files).build_jvm_based_bundle(Some(debug_id)) {
Ok(tempfile) => fs::copy(tempfile.path(), &out),
Err(e) => bail!("Unable to create source bundle: {}", e),
};
match copy_result {
Ok(_) => println!("Created {}", out.to_string_lossy()),
Err(e) => bail!("Unable to write source bundle: {}", e)
}
Ok(())
adinauer marked this conversation as resolved.
Show resolved Hide resolved
} else {
adinauer marked this conversation as resolved.
Show resolved Hide resolved
bail!("Given path is not a directory: {}", path.to_string_lossy())
}
}
1 change: 1 addition & 0 deletions src/commands/debug_files/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ fn find_ids(
DifType::SourceBundle => find_ids_for_sourcebundle(&dirent, &remaining),
DifType::Breakpad => find_ids_for_breakpad(&dirent, &remaining),
DifType::Proguard => find_ids_for_proguard(&dirent, &proguard_uuids),
DifType::JvmBased => find_ids_for_sourcebundle(&dirent, &remaining),
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 not sure about the whole jvmbased naming. We dont use PdbBased, ElfBased, ProguardBasd etc. Might use JvmBundle or just Jvm instead? wdyt @Swatinem

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I guess there's little risk in people actually thinking they need to upload a JVM. Can rename it here and in other PRs as this was suggested before. Happy to rename and make it easier to use.

Copy link
Member

Choose a reason for hiding this comment

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

Well, naming is only one of the hardest things in software… :-D I’m okay with the naming. It clearly states that we have are dealing with any of the myriad of languages that compile to java bytecode and run on the jvm.

DifType::Wasm => None,
})
.flatten()
Expand Down
2 changes: 2 additions & 0 deletions src/commands/debug_files/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use clap::{ArgMatches, Command};

pub mod bundle_sources;
pub mod check;
pub mod create_jvm_based_bundle;
adinauer marked this conversation as resolved.
Show resolved Hide resolved
pub mod find;
pub mod print_sources;
pub mod upload;
Expand All @@ -11,6 +12,7 @@ macro_rules! each_subcommand {
($mac:ident) => {
$mac!(bundle_sources);
$mac!(check);
$mac!(create_jvm_based_bundle);
$mac!(find);
$mac!(print_sources);
$mac!(upload);
Expand Down
1 change: 1 addition & 0 deletions src/commands/debug_files/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
"pe" => upload.filter_format(DifFormat::Object(FileFormat::Pe)),
"sourcebundle" => upload.filter_format(DifFormat::Object(FileFormat::SourceBundle)),
"portablepdb" => upload.filter_format(DifFormat::Object(FileFormat::PortablePdb)),
"jvmbased" => upload.filter_format(DifFormat::Object(FileFormat::SourceBundle)),
"bcsymbolmap" => {
upload.filter_format(DifFormat::BcSymbolMap);
upload.filter_format(DifFormat::PList)
Expand Down
6 changes: 6 additions & 0 deletions src/utils/dif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub enum DifType {
Pdb,
PortablePdb,
Wasm,
JvmBased,
}

impl DifType {
Expand All @@ -35,6 +36,7 @@ impl DifType {
DifType::Breakpad => "breakpad",
DifType::Proguard => "proguard",
DifType::Wasm => "wasm",
DifType::JvmBased => "jvmbased",
}
}

Expand All @@ -49,6 +51,7 @@ impl DifType {
DifType::Breakpad,
DifType::Proguard,
DifType::Wasm,
DifType::JvmBased,
]
}

Expand All @@ -63,6 +66,7 @@ impl DifType {
"breakpad",
"proguard",
"wasm",
"jvmbased",
]
}
}
Expand All @@ -87,6 +91,7 @@ impl str::FromStr for DifType {
"breakpad" => Ok(DifType::Breakpad),
"proguard" => Ok(DifType::Proguard),
"wasm" => Ok(DifType::Wasm),
"jvmbased" => Ok(DifType::JvmBased),
_ => bail!("Invalid debug info file type"),
}
}
Expand Down Expand Up @@ -251,6 +256,7 @@ impl DifFile<'static> {
Some(DifType::Wasm) => DifFile::open_object(path, FileFormat::Wasm),
Some(DifType::Breakpad) => DifFile::open_object(path, FileFormat::Breakpad),
Some(DifType::Proguard) => DifFile::open_proguard(path),
Some(DifType::JvmBased) => DifFile::open_object(path, FileFormat::SourceBundle),
None => DifFile::try_open(path),
}
}
Expand Down
21 changes: 17 additions & 4 deletions src/utils/file_upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ impl<'a> FileUpload<'a> {
.map_or(DEFAULT_CONCURRENCY, |o| usize::from(o.concurrency));
upload_files_parallel(self.context, &self.files, concurrency)
}

pub fn build_jvm_based_bundle(&self, debug_id: Option<DebugId>) -> Result<TempFile> {
build_artifact_bundle(self.context, &self.files, debug_id)
}
}

fn upload_files_parallel(
Expand Down Expand Up @@ -274,7 +278,7 @@ fn upload_files_chunked(
files: &SourceFiles,
options: &ChunkUploadOptions,
) -> Result<()> {
let archive = build_artifact_bundle(context, files)?;
let archive = build_artifact_bundle(context, files, None)?;

let progress_style =
ProgressStyle::default_spinner().template("{spinner} Optimizing bundle for upload...");
Expand Down Expand Up @@ -396,7 +400,11 @@ fn build_debug_id(files: &SourceFiles) -> DebugId {
DebugId::from_uuid(uuid::Builder::from_sha1_bytes(sha1_bytes).into_uuid())
}

fn build_artifact_bundle(context: &UploadContext, files: &SourceFiles) -> Result<TempFile> {
fn build_artifact_bundle(
context: &UploadContext,
files: &SourceFiles,
debug_id: Option<DebugId>,
) -> Result<TempFile> {
let progress_style = ProgressStyle::default_bar().template(
"{prefix:.dim} Bundling files for upload... {msg:.dim}\
\n{wide_bar} {pos}/{len}",
Expand All @@ -409,8 +417,13 @@ fn build_artifact_bundle(context: &UploadContext, files: &SourceFiles) -> Result
let archive = TempFile::create()?;
let mut bundle = SourceBundleWriter::start(BufWriter::new(archive.open()?))?;

// artifact bundles get a random UUID as debug id
bundle.set_attribute("debug_id", build_debug_id(files).to_string());
match debug_id {
Some(id) => bundle.set_attribute("debug_id", id.to_string()),
None => {
// artifact bundles get a random UUID as debug id
bundle.set_attribute("debug_id", build_debug_id(files).to_string())
}
};
adinauer marked this conversation as resolved.
Show resolved Hide resolved
if let Some(note) = context.note {
bundle.set_attribute("note", note.to_owned());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --help
? success
Create a source bundle for the given JVM based source files (e.g. Java, Kotlin, ...)

Usage: sentry-cli[EXE] debug-files create-jvm-based-bundle [OPTIONS] --output <PATH> --debug-id <UUID> <PATH>

Arguments:
<PATH> The directory containing source files to bundle.

Options:
-o, --org <ORG> The organization slug
--header <KEY:VALUE> Custom headers that should be attached to all requests
in key:value format.
-p, --project <PROJECT> The project slug.
--auth-token <AUTH_TOKEN> Use the given Sentry auth token.
--output <PATH> The path to the output folder.
--debug-id <UUID> Debug ID to use for the source bundle.
--log-level <LOG_LEVEL> Set the log output verbosity. [possible values: trace, debug, info,
warn, error]
--quiet Do not print any output while preserving correct exit code. This
flag is currently implemented only for selected subcommands.
[aliases: silent]
-h, --help Print help

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --help
? success
Create a source bundle for the given JVM based source files (e.g. Java, Kotlin, ...)

Usage: sentry-cli debug-files create-jvm-based-bundle [OPTIONS] --output <PATH> --debug-id <UUID> <PATH>

Arguments:
<PATH> The directory containing source files to bundle.

Options:
-o, --org <ORG> The organization slug
--header <KEY:VALUE> Custom headers that should be attached to all requests
in key:value format.
-p, --project <PROJECT> The project slug.
--auth-token <AUTH_TOKEN> Use the given Sentry auth token.
--output <PATH> The path to the output folder.
--debug-id <UUID> Debug ID to use for the source bundle.
--log-level <LOG_LEVEL> Set the log output verbosity. [possible values: trace, debug, info,
warn, error]
--quiet Do not print any output while preserving correct exit code. This
flag is currently implemented only for selected subcommands.
[aliases: silent]
-h, --help Print help

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --output . --debug-id 48dee70b-4f3f-4a49-9223-de441738f7cd empty-dir
? success
> Found 0 files
> Bundled 0 files for upload
Created ./48dee70b-4f3f-4a49-9223-de441738f7cd.zip

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --output `cwd` --debug-id 59144E0E-52C4-41F8-9111-0D6F8F14905B tests/integration/_fixtures/jvmbased/io/sentry/sample/MainActivity.java
? failed
error: Given path is not a directory: tests/integration/_fixtures/jvmbased/io/sentry/sample/MainActivity.java

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --output `cwd` --debug-id 7A575F05-0585-4DB0-95DE-D0C032D7C707 tests/integration/_fixtures/i-do-not-exist
? failed
error: Given path does not exist: tests/integration/_fixtures/i-do-not-exist

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --output . --debug-id not-a-valid-uuid io
? failed
error: Given debug_id is invalid: not-a-valid-uuid

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --output ./file.txt --debug-id D384DC3B-AB2F-4DC7-903D-2C851E27E094 tests/integration/_fixtures/jvmbased/
? failed
> Found 2 files
> Bundled 2 files for upload
error: Unable to write source bundle: The system cannot find the path specified. (os error 3)

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --output ./file.txt --debug-id D384DC3B-AB2F-4DC7-903D-2C851E27E094 ./io
? failed
> Found 2 files
> Bundled 2 files for upload
error: Unable to write source bundle: Not a directory (os error 20)

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --output i-do-not-exist --debug-id 0B693ABA-531C-4EB6-99E4-B7320C3C85DA tests/integration/_fixtures/jvmbased
? failed
> Found 2 files
> Bundled 2 files for upload
error: Unable to write source bundle: The system cannot find the path specified. (os error 3)

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --output i-do-not-exist --debug-id 0B693ABA-531C-4EB6-99E4-B7320C3C85DA tests/integration/_fixtures/jvmbased
? failed
> Found 2 files
> Bundled 2 files for upload
error: Unable to write source bundle: No such file or directory (os error 2)

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
```
$ sentry-cli debug-files create-jvm-based-bundle --output . --debug-id a4368a48-0880-40d7-9a26-c9ef5a84d156 ./io
? success
> Found 2 files
> Bundled 2 files for upload
Created ./a4368a48-0880-40d7-9a26-c9ef5a84d156.zip

```
Loading