Skip to content

Conversation

jjbayer
Copy link
Member

@jjbayer jjbayer commented Jun 12, 2024

#3633 introduced separate feature flags for span extraction and span metrics, but this meant that there was no usage metric for spans that were extracted without common span metrics. We need the usage metric for accounting purposes.

@jjbayer jjbayer marked this pull request as ready for review June 12, 2024 10:16
@jjbayer jjbayer requested a review from a team as a code owner June 12, 2024 10:16
@jjbayer jjbayer requested a review from phacops June 12, 2024 10:17
// If there are any spans in the system, extract the usage metric for them:
let any_spans = features.has(Feature::ExtractSpansFromEvent)
|| features.has(Feature::StandaloneSpanIngestion)
|| features.has(Feature::ExtractCommonSpanMetricsFromEvent);
Copy link
Contributor

@phacops phacops Jun 12, 2024

Choose a reason for hiding this comment

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

You could add Feature::ExtractAddonsSpanMetricsFromEvent as well even though it's meant to be enabled only if common is enabled.

pub fn as_str(&self) -> Option<&'a str> {
match self {
Self::String(value) => Some(value),

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

@jjbayer jjbayer merged commit 409ea67 into master Jun 12, 2024
@jjbayer jjbayer deleted the fix/spans-always-usage branch June 12, 2024 11:49
jjbayer added a commit that referenced this pull request Jun 12, 2024
#3719 enabled
`c:spans/usage@none` for plans that do not extract other span metrics.
We also need to allow the namespace otherwise they will be dropped with
outcome `filtered:disabled-namespace`.
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.

3 participants