Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(metrics): Remove timestamp from text protocol #972

Merged
merged 13 commits into from
Apr 12, 2021

Conversation

jjbayer
Copy link
Member

@jjbayer jjbayer commented Apr 6, 2021

This pull request

  • removes the optional timestamp ('12345678.9) from the text protocol,
  • adds an optional timestamp to ItemHeaders, which can be submitted as date string or number,
  • removes the unused Metric::serialize() function
    (cannot serialize to text protocol without dropping timestamp).

@jjbayer jjbayer requested review from a team and jan-auer April 6, 2021 10:03
//! Clients submit metrics in a [text-based protocol](Metric) based on StatsD. A sample submission
//! looks like this:
//! Clients submit metrics in envelopes containing a [text-based protocol](Metric) based on StatsD.
//! A sample submission looks like this:
Copy link
Member

Choose a reason for hiding this comment

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

Can we split this up into the text-based protocol, and then the Envelope protocol, like we had in Notion? It's good to keep the examples to the text portion of it, since that is StatsD compatible and we might want to introduce a new endpoint that takes this as plain text input.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@@ -26,6 +26,9 @@ sentry-types = "0.20.0"
schemars = { version = "0.8.1", features = ["uuid", "chrono"], optional = true }
serde = { version = "1.0.114", features = ["derive"] }

[dev-dependencies]
serde_test = ""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
serde_test = ""
serde_test = "1.0.125"

CHANGELOG.md Outdated
@@ -18,7 +19,7 @@
- Emit the `quantity` field for outcomes of events. This field describes the total size in bytes for attachments or the event count for all other categories. A separate outcome is emitted for attachments in a rejected envelope, if any, in addition to the event outcome. ([#942](https://github.com/getsentry/relay/pull/942))
- Add experimental metrics ingestion without bucketing or pre-aggregation. ([#948](https://github.com/getsentry/relay/pull/948))
- Skip serializing some null values in frames interface. ([#944](https://github.com/getsentry/relay/pull/944))
- Add experimental metrics ingestion with bucketing and pre-aggregation. ([#948](https://github.com/getsentry/relay/pull/948), [#952](https://github.com/getsentry/relay/pull/952), [#958](https://github.com/getsentry/relay/pull/958), [#966](https://github.com/getsentry/relay/pull/966), [#969](https://github.com/getsentry/relay/pull/969))
- Add experimental metrics ingestion with bucketing and pre-aggregation. ([#948](https://github.com/getsentry/relay/pull/948), [#952](https://github.com/getsentry/relay/pull/952), [#958](https://github.com/getsentry/relay/pull/958), [#966](https://github.com/getsentry/relay/pull/966), [#969](https://github.com/getsentry/relay/pull/969)).
Copy link
Member

Choose a reason for hiding this comment

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

This should be reverted.

@jjbayer jjbayer merged commit 66959ca into master Apr 12, 2021
@jjbayer jjbayer deleted the feat/metrics-remove-timestamp branch April 12, 2021 14:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants