Fix timestamp handling in remote_write#21166
Merged
ChrsMark merged 4 commits intoelastic:masterfrom Sep 23, 2020
Merged
Conversation
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Contributor
|
Pinging @elastic/integrations-platforms (Team:Platforms) |
Signed-off-by: chrismark <chrismarkou92@gmail.com>
exekias
reviewed
Sep 18, 2020
Comment on lines
+60
to
+63
| // join metrics with same labels and same timestamp in a single event | ||
| timeStampedLabels := labels.Clone() | ||
| timeStampedLabels.Put("timestamp", metric.Timestamp.Time()) | ||
| labelsHash := timeStampedLabels.String() |
Contributor
There was a problem hiding this comment.
I would try to avoid cloning to save memory and cycles. How about appending the timestamp to the labelsHash?
Something like sampleHash := labels.String() + metric.Timestamp.Time()
exekias
suggested changes
Sep 18, 2020
Contributor
exekias
left a comment
There was a problem hiding this comment.
Thank you for taking on this! did you happen to reproduce the issue?
Contributor
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
Member
Author
Not with a real Prometheus setup but the testcase https://github.com/elastic/beats/pull/21166/files#diff-08a773efe40f622118ed7a6596ea3adaR45 reproduces exactly a scenario where a metric sent twice in a chunk could be saved only once from us due to the bad timestamp handling. |
Signed-off-by: chrismark <chrismarkou92@gmail.com>
exekias
approved these changes
Sep 22, 2020
ChrsMark
added a commit
to ChrsMark/beats
that referenced
this pull request
Sep 23, 2020
(cherry picked from commit d2a8922)
3 tasks
ChrsMark
added a commit
to ChrsMark/beats
that referenced
this pull request
Sep 23, 2020
(cherry picked from commit d2a8922)
3 tasks
ChrsMark
added a commit
that referenced
this pull request
Sep 23, 2020
ChrsMark
added a commit
that referenced
this pull request
Sep 23, 2020
v1v
added a commit
to v1v/beats
that referenced
this pull request
Sep 24, 2020
…ne-2.0-arm * upstream/master: (29 commits) Fix librpm installation in auditbeat build (elastic#21239) Fix prometheus default config (elastic#21253) Fix dev guide test command (elastic#21254) Move aws lambda metricset to GA (elastic#21255) [Docs] Typo in table syntax (elastic#20227) [ECS] Adds related.hosts to capture all hostnames and host identifiers on an event. (elastic#21160) Add recursive split to httpjson (elastic#21214) [DOCS] Add beat specific start widgets (elastic#21217) Fix timestamp handling in remote_write (elastic#21166) Fix aws, azure and googlecloud compute dashboards (elastic#21098) Add acceptable event log keys to winlog (elastic#21205) Add elastic-agent to gitignore (elastic#21219) Add cloudfoundry tags to events (elastic#21177) [Ingest Manager] Agent includes pgp file (elastic#19480) Add compatibility note about ingress-controller-v0.34.1 (elastic#21209) [Ingest Manager] Support for UPGRADE_ACTION (elastic#21002) Fix libbeat.output.*.bytes metrics of Elasticsearch output (elastic#21197) [packaging] use docker.elastic.co/ubi8/ubi-minimal (elastic#21154) Add host inventory metrics to system module (elastic#20415) [Filebeat][Azure Module] Fixing event.outcome from result_type issue (elastic#20998) ...
v1v
added a commit
to v1v/beats
that referenced
this pull request
Sep 24, 2020
…ne-2.0 * upstream/master: (33 commits) Stop running agent container as root by default (elastic#21213) Stop running auditbeat container as root by default (elastic#21202) Fix autodiscover flaky tests (elastic#21242) [Ingest Manager] Enabled dev builds (elastic#21241) Fix librpm installation in auditbeat build (elastic#21239) Fix prometheus default config (elastic#21253) Fix dev guide test command (elastic#21254) Move aws lambda metricset to GA (elastic#21255) [Docs] Typo in table syntax (elastic#20227) [ECS] Adds related.hosts to capture all hostnames and host identifiers on an event. (elastic#21160) Add recursive split to httpjson (elastic#21214) [DOCS] Add beat specific start widgets (elastic#21217) Fix timestamp handling in remote_write (elastic#21166) Fix aws, azure and googlecloud compute dashboards (elastic#21098) Add acceptable event log keys to winlog (elastic#21205) Add elastic-agent to gitignore (elastic#21219) Add cloudfoundry tags to events (elastic#21177) [Ingest Manager] Agent includes pgp file (elastic#19480) Add compatibility note about ingress-controller-v0.34.1 (elastic#21209) [Ingest Manager] Support for UPGRADE_ACTION (elastic#21002) ...
v1v
added a commit
to v1v/beats
that referenced
this pull request
Sep 28, 2020
* upstream/master: (417 commits) libbeat/cmd/instance: report cgroup stats (elastic#21113) Configurable index template loading (elastic#21212) [Ingest Manager] Thread safe sorted set (elastic#21290) Change mirror of kafka download (elastic#19645) [Ingest manager] Copy Action store on upgrade (elastic#21298) [CI] Pipeline 2.0 for monorepos (elastic#20104) Stop running agent container as root by default (elastic#21213) Stop running auditbeat container as root by default (elastic#21202) Fix autodiscover flaky tests (elastic#21242) [Ingest Manager] Enabled dev builds (elastic#21241) Fix librpm installation in auditbeat build (elastic#21239) Fix prometheus default config (elastic#21253) Fix dev guide test command (elastic#21254) Move aws lambda metricset to GA (elastic#21255) [Docs] Typo in table syntax (elastic#20227) [ECS] Adds related.hosts to capture all hostnames and host identifiers on an event. (elastic#21160) Add recursive split to httpjson (elastic#21214) [DOCS] Add beat specific start widgets (elastic#21217) Fix timestamp handling in remote_write (elastic#21166) Fix aws, azure and googlecloud compute dashboards (elastic#21098) ...
v1v
added a commit
to v1v/beats
that referenced
this pull request
Sep 28, 2020
* upstream/master: (399 commits) libbeat/cmd/instance: report cgroup stats (elastic#21113) Configurable index template loading (elastic#21212) [Ingest Manager] Thread safe sorted set (elastic#21290) Change mirror of kafka download (elastic#19645) [Ingest manager] Copy Action store on upgrade (elastic#21298) [CI] Pipeline 2.0 for monorepos (elastic#20104) Stop running agent container as root by default (elastic#21213) Stop running auditbeat container as root by default (elastic#21202) Fix autodiscover flaky tests (elastic#21242) [Ingest Manager] Enabled dev builds (elastic#21241) Fix librpm installation in auditbeat build (elastic#21239) Fix prometheus default config (elastic#21253) Fix dev guide test command (elastic#21254) Move aws lambda metricset to GA (elastic#21255) [Docs] Typo in table syntax (elastic#20227) [ECS] Adds related.hosts to capture all hostnames and host identifiers on an event. (elastic#21160) Add recursive split to httpjson (elastic#21214) [DOCS] Add beat specific start widgets (elastic#21217) Fix timestamp handling in remote_write (elastic#21166) Fix aws, azure and googlecloud compute dashboards (elastic#21098) ...
v1v
added a commit
to v1v/beats
that referenced
this pull request
Sep 28, 2020
* upstream/master: (60 commits) libbeat/cmd/instance: report cgroup stats (elastic#21113) Configurable index template loading (elastic#21212) [Ingest Manager] Thread safe sorted set (elastic#21290) Change mirror of kafka download (elastic#19645) [Ingest manager] Copy Action store on upgrade (elastic#21298) [CI] Pipeline 2.0 for monorepos (elastic#20104) Stop running agent container as root by default (elastic#21213) Stop running auditbeat container as root by default (elastic#21202) Fix autodiscover flaky tests (elastic#21242) [Ingest Manager] Enabled dev builds (elastic#21241) Fix librpm installation in auditbeat build (elastic#21239) Fix prometheus default config (elastic#21253) Fix dev guide test command (elastic#21254) Move aws lambda metricset to GA (elastic#21255) [Docs] Typo in table syntax (elastic#20227) [ECS] Adds related.hosts to capture all hostnames and host identifiers on an event. (elastic#21160) Add recursive split to httpjson (elastic#21214) [DOCS] Add beat specific start widgets (elastic#21217) Fix timestamp handling in remote_write (elastic#21166) Fix aws, azure and googlecloud compute dashboards (elastic#21098) ...
leweafan
pushed a commit
to leweafan/beats
that referenced
this pull request
Apr 28, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes how we handle timestamps when we process Prometheus Samples using remote_write. Before this change it could be possible that we miss some metrics in case of having the same metric in the same received chunk (with different timestamps). Metrics with different timestamps should go in different final events and hence
timestampshould be taken into account when grouping metrics into events.Why is it important?
To avoid losing data due to wrong grouping.
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.