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

feat: Use checksum to dedupe uploaded release artifacts #1275

Merged
merged 2 commits into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/commands/files/upload.rs
Expand Up @@ -151,6 +151,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
ty: SourceFileType::Source,
headers: headers.clone(),
messages: vec![],
already_uploaded: false,
},
)
})
Expand Down
11 changes: 10 additions & 1 deletion src/commands/sourcemaps/upload.rs
@@ -1,9 +1,11 @@
use std::path::PathBuf;
use std::str::FromStr;

use anyhow::Result;
use anyhow::{format_err, Result};
use clap::{Arg, ArgMatches, Command};
use glob::{glob_with, MatchOptions};
use log::{debug, warn};
use sha1_smol::Digest;

use crate::api::{Api, NewRelease};
use crate::config::Config;
Expand Down Expand Up @@ -303,6 +305,13 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
let api = Api::current();
let mut processor = SourceMapProcessor::new();

for artifact in api.list_release_files(&org, Some(&project), &version)? {
let checksum = Digest::from_str(&artifact.sha1)
.map_err(|_| format_err!("Invalid artifact checksum"))?;

processor.add_already_uploaded_source(checksum);
}

if matches.is_present("bundle") && matches.is_present("bundle_sourcemap") {
process_sources_from_bundle(matches, &mut processor)?;
} else {
Expand Down
9 changes: 8 additions & 1 deletion src/utils/file_upload.rs
Expand Up @@ -12,14 +12,15 @@ use console::style;
use parking_lot::RwLock;
use rayon::prelude::*;
use rayon::ThreadPoolBuilder;
use sha1_smol::Digest;
use symbolic::common::ByteView;
use symbolic::debuginfo::sourcebundle::{SourceBundleWriter, SourceFileInfo, SourceFileType};
use url::Url;

use crate::api::{Api, ChunkUploadCapability, ChunkUploadOptions, FileContents, ProgressBarMode};
use crate::constants::DEFAULT_MAX_WAIT;
use crate::utils::chunks::{upload_chunks, Chunk, ASSEMBLE_POLL_INTERVAL};
use crate::utils::fs::{get_sha1_checksums, TempFile};
use crate::utils::fs::{get_sha1_checksum, get_sha1_checksums, TempFile};
use crate::utils::progress::{ProgressBar, ProgressStyle};

/// Fallback concurrency for release file uploads.
Expand Down Expand Up @@ -56,9 +57,15 @@ pub struct ReleaseFile {
pub ty: SourceFileType,
pub headers: Vec<(String, String)>,
pub messages: Vec<(LogLevel, String)>,
pub already_uploaded: bool,
}

impl ReleaseFile {
/// Calculates and returns the SHA1 checksum of the file.
pub fn checksum(&self) -> Result<Digest> {
get_sha1_checksum(&*self.contents)
}

pub fn log(&mut self, level: LogLevel, msg: String) {
self.messages.push((level, msg));
}
Expand Down
31 changes: 31 additions & 0 deletions src/utils/sourcemaps.rs
Expand Up @@ -9,6 +9,7 @@ use anyhow::{bail, Error, Result};
use console::style;
use indicatif::ProgressStyle;
use log::{debug, info, warn};
use sha1_smol::Digest;
use symbolic::debuginfo::sourcebundle::SourceFileType;
use url::Url;

Expand Down Expand Up @@ -147,6 +148,7 @@ fn guess_sourcemap_reference(sourcemaps: &HashSet<String>, min_url: &str) -> Res

pub struct SourceMapProcessor {
pending_sources: HashSet<(String, ReleaseFileMatch)>,
already_uploaded_sources: Vec<Digest>,
sources: ReleaseFiles,
}

Expand All @@ -162,6 +164,7 @@ impl SourceMapProcessor {
pub fn new() -> SourceMapProcessor {
SourceMapProcessor {
pending_sources: HashSet::new(),
already_uploaded_sources: Vec::new(),
sources: HashMap::new(),
}
}
Expand All @@ -172,6 +175,11 @@ impl SourceMapProcessor {
Ok(())
}

/// Adds an already uploaded sources checksum.
pub fn add_already_uploaded_source(&mut self, checksum: Digest) {
self.already_uploaded_sources.push(checksum);
}

fn flush_pending_sources(&mut self) {
if self.pending_sources.is_empty() {
return;
Expand All @@ -191,6 +199,7 @@ impl SourceMapProcessor {
);
for (url, mut file) in self.pending_sources.drain() {
pb.set_message(&url);

let ty = if sourcemap::is_sourcemap_slice(&file.contents) {
SourceFileType::SourceMap
} else if file
Expand Down Expand Up @@ -232,6 +241,7 @@ impl SourceMapProcessor {
ty,
headers: vec![],
messages: vec![],
already_uploaded: false,
},
);
pb.inc(1);
Expand Down Expand Up @@ -268,6 +278,14 @@ impl SourceMapProcessor {
sect = Some(source.ty);
}

if source.already_uploaded {
println!(
" {}",
style(format!("{} (skipped; already uploaded)", &source.url)).yellow()
);
continue;
}

if source.ty == SourceFileType::MinifiedSource {
if let Some(sm_ref) = get_sourcemap_ref(source) {
let url = sm_ref.get_url();
Expand Down Expand Up @@ -401,6 +419,7 @@ impl SourceMapProcessor {
ty: SourceFileType::MinifiedSource,
headers: vec![],
messages: vec![],
already_uploaded: false,
},
);

Expand All @@ -418,6 +437,7 @@ impl SourceMapProcessor {
ty: SourceFileType::SourceMap,
headers: vec![],
messages: vec![],
already_uploaded: false,
},
);
}
Expand Down Expand Up @@ -527,9 +547,20 @@ impl SourceMapProcessor {
Ok(())
}

fn flag_uploaded_sources(&mut self) {
for source in self.sources.values_mut() {
if let Ok(checksum) = &source.checksum() {
if self.already_uploaded_sources.contains(checksum) {
source.already_uploaded = true;
}
}
}
}

/// Uploads all files
pub fn upload(&mut self, context: &UploadContext<'_>) -> Result<()> {
self.flush_pending_sources();
self.flag_uploaded_sources();
let mut uploader = ReleaseFileUpload::new(context);
uploader.files(&self.sources);
uploader.upload()?;
Expand Down
96 changes: 96 additions & 0 deletions tests/integration/_cases/sourcemaps/sourcemaps-upload-help.trycmd
@@ -0,0 +1,96 @@
```
$ sentry-cli sourcemaps upload --help
? success
sentry-cli[EXE]-sourcemaps-upload
Upload sourcemaps for a release.

USAGE:
sentry-cli[EXE] sourcemaps upload [OPTIONS] [PATHS]...

ARGS:
<PATHS>... The files to upload.

OPTIONS:
--auth-token <AUTH_TOKEN>
Use the given Sentry auth token.

--bundle <BUNDLE>
Path to the application bundle (indexed, file, or regular)

--bundle-sourcemap <BUNDLE_SOURCEMAP>
Path to the bundle sourcemap

-d, --dist <DISTRIBUTION>
Optional distribution identifier for the sourcemaps.

-h, --help
Print help information

--header <KEY:VALUE>
Custom headers that should be attached to all requests
in key:value format.

-i, --ignore <IGNORE>
Ignores all files and folders matching the given glob

-I, --ignore-file <IGNORE_FILE>
Ignore all files and folders specified in the given ignore file, e.g. .gitignore.

--log-level <LOG_LEVEL>
Set the log output verbosity. [possible values: trace, debug, info, warn, error]

--no-rewrite
Disables rewriting of matching sourcemaps. By default the tool will rewrite sources, so
that indexed maps are flattened and missing sources are inlined if possible.
This fundamentally changes the upload process to be based on sourcemaps and minified
files exclusively and comes in handy for setups like react-native that generate
sourcemaps that would otherwise not work for sentry.

--no-sourcemap-reference
Disable emitting of automatic sourcemap references.
By default the tool will store a 'Sourcemap' header with minified files so that
sourcemaps are located automatically if the tool can detect a link. If this causes
issues it can be disabled.

-o, --org <ORG>
The organization slug

-p, --project <PROJECT>
The project slug.

--quiet
Do not print any output while preserving correct exit code. This flag is currently
implemented only for selected subcommands. [aliases: silent]

-r, --release <RELEASE>
The release slug.

--strip-common-prefix
Similar to --strip-prefix but strips the most common prefix on all sources references.

--strip-prefix <PREFIX>
Strips the given prefix from all sources references inside the upload sourcemaps (paths
used within the sourcemap content, to map minified code to it's original source). Only
sources that start with the given prefix will be stripped.
This will not modify the uploaded sources paths. To do that, point the upload or
upload-sourcemaps command to a more precise directory instead.

-u, --url-prefix <PREFIX>
The URL prefix to prepend to all filenames.

--url-suffix <SUFFIX>
The URL suffix to append to all filenames.

--validate
Enable basic sourcemap validation.

--wait
Wait for the server to fully process uploaded files.

-x, --ext <EXT>
Set the file extensions that are considered for upload. This overrides the default
extensions. To add an extension, all default extensions must be repeated. Specify once
per extension.
Defaults to: `--ext=js --ext=map --ext=jsbundle --ext=bundle`

```
@@ -0,0 +1,20 @@
```
$ sentry-cli sourcemaps upload tests/integration/_fixtures/bundle.min.js.map --release=wat-release
? success
> Found 1 release file
> Analyzing 1 sources
> Rewriting sources
> Adding source map references
> Bundled 1 file for upload
> Uploaded release files to Sentry
> File upload complete (processing pending on server)
> Organization: wat-org
> Project: wat-project
> Release: wat-release
> Dist: None

Source Map Upload Report
Source Maps
~/bundle.min.js.map (skipped; already uploaded)

```
@@ -0,0 +1,20 @@
```
$ sentry-cli sourcemaps upload tests/integration/_fixtures/bundle.min.js.map --release=wat-release
? success
> Found 1 release file
> Analyzing 1 sources
> Rewriting sources
> Adding source map references
> Bundled 1 file for upload
> Uploaded release files to Sentry
> File upload complete (processing pending on server)
> Organization: wat-org
> Project: wat-project
> Release: wat-release
> Dist: None

Source Map Upload Report
Source Maps
~/bundle.min.js.map

```
12 changes: 12 additions & 0 deletions tests/integration/_cases/sourcemaps/sourcemaps-upload.trycmd
@@ -0,0 +1,12 @@
```
$ sentry-cli sourcemaps upload
? failed
error: The following required arguments were not provided:
<PATHS>...

USAGE:
sentry-cli[EXE] sourcemaps upload <PATHS>...

For more information try --help

```
1 change: 1 addition & 0 deletions tests/integration/sourcemaps/mod.rs
Expand Up @@ -2,6 +2,7 @@ use crate::integration::register_test;

mod explain;
mod resolve;
mod upload;

#[test]
fn command_sourcemaps_help() {
Expand Down