Skip to content

fix: Fix kafka topic default#3350

Merged
lynnagara merged 3 commits intomasterfrom
fix-topic-default
Mar 28, 2024
Merged

fix: Fix kafka topic default#3350
lynnagara merged 3 commits intomasterfrom
fix-topic-default

Conversation

@lynnagara
Copy link
Copy Markdown
Member

should be ingest-performance-metrics not ingest-generic metrics

INC-696

should be ingest-performance-metrics not ingest-generic metrics

INC-696
@lynnagara lynnagara requested a review from a team as a code owner March 27, 2024 23:48
pub metrics_sessions: TopicAssignment,
/// Topic name for all other kinds of metrics. Defaults to the assignment of `metrics`.
#[serde(alias = "metrics_transactions", alias = "ingest-generic-metrics")]
#[serde(alias = "metrics_transactions", alias = "ingest-performance-metrics")]
Copy link
Copy Markdown
Member

@Dav1dde Dav1dde Mar 28, 2024

Choose a reason for hiding this comment

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

Are we sure it's not both, e.g. single tenant?

In the incident channel you said it worked on single tenant, if I search for ingest-generic-metrics I do find some results in the ops repo.

Copy link
Copy Markdown
Member Author

@lynnagara lynnagara Mar 28, 2024

Choose a reason for hiding this comment

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

Yeah, the key here is the logical or default topic, what can get overridden to in each environment (such as different single tenants) via the TopicAssignment value.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I need to find a way to validate these against what we have registered in https://github.com/getsentry/sentry-kafka-schemas/blob/main/topics/ingest-performance-metrics.yaml to avoid this happening in the future.

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.

We're already pulling in sentry-kafka-schemas, can probably use it to validate the topics in a test?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sounds good, I need to spend some time to figure how to make this work + also try to remove the duplication of default topics in the file -- will follow up with something shortly.

@lynnagara lynnagara enabled auto-merge (squash) March 28, 2024 17:41
@lynnagara lynnagara merged commit 1daaa14 into master Mar 28, 2024
@lynnagara lynnagara deleted the fix-topic-default branch March 28, 2024 18:09
jan-auer added a commit that referenced this pull request Apr 3, 2024
* master:
  feat(metric-stats): Report cardinality to metric stats (#3360)
  release: 0.8.56
  fix(perfscore): Adds span op tag to perf score totals (#3326)
  ref(profiles): Return retention_days as part of the Kafka message (#3362)
  ref(filter): Add GTmetrix to the list of web crawlers (#3363)
  fix: Fix kafka topic default (#3350)
  ref(normalization): Remove duplicated normalization (#3355)
  feat(feedback): Emit outcomes for user feedback events (#3026)
  feat(cardinality): Implement cardinality reporting (#3342)
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.

4 participants