Skip to content

ref: Implement ingest_path attribute - #6402

Open
pierceroberts wants to merge 2 commits into
getsentry:masterfrom
pierceroberts:proberts-imple-ingest-path
Open

pierceroberts wants to merge 2 commits into
getsentry:masterfrom
pierceroberts:proberts-imple-ingest-path

Conversation

@pierceroberts

Copy link
Copy Markdown
Contributor

This is related to #6071
and is my first attempt at getting this implemented.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@pierceroberts
pierceroberts requested a review from a team as a code owner September 21, 2026 05:58

@loewenheim loewenheim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi, thank you for the contribution! This looks sensible. However, you will need to rebase on top of master (or merge it) because some things related to Config have changed in the meantime.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can temporarily add your new attribute as a constant in this PR, then open a PR to conventions adding the attribute, then update this PR.

"sentry.description": {
"type": "string",
"value": "secret123"
"value": "[Filtered]"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably a result of the sentry-conventions confusion.

Comment on lines +11 to +16
fn format_relay_ingest_path_prefix(version: &str, git_revision: Option<&str>) -> String {
match git_revision {
Some(revision) if !revision.is_empty() => format!("{version}@[{revision}]"),
_ => version.to_owned(),
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd fold this into format_relay_ingest_path_entry. Having it available separately in current_relay_ingest_path_entry doesn't save you that much, you have to call format! on every access anyway.

Comment on lines +55 to +58
.map(|value| match value.value()? {
Value::String(value) => Some(value.clone()),
_ => None,
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
.map(|value| match value.value()? {
Value::String(value) => Some(value.clone()),
_ => None,
})
.map(|value| Some(value.as_str()?.to_owned()))

ingest_path.push(current_entry.to_owned());
}

let attributes = attributes.get_or_insert_with(Default::default);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This line can move to the start of the function.

This branch has not been deployed

No deployments
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.

2 participants