Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Allow to schedule disconnected container entities for perf capture #16302

Conversation

Ladas
Copy link
Contributor

@Ladas Ladas commented Oct 25, 2017

Collect metrics for archived Container entities. We need this
for shortlived entities (e.g. pods), to make sure the entity
is not discovered and archived before the metrics capture
was planned. The amount of time for looking back for archived
items is configurable and by default is 8h.

Partially fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1506671

@Ladas
Copy link
Contributor Author

Ladas commented Oct 25, 2017

@miq-bot assign @agrare

cc @cben @simon3z @yaacov btw. I am seeing a strange behavior with Hawkular. When I delete the pod, Hawkular stops giving me samples, while before delete, it returned samples. Any idea why? Haven't tested Prometheus.

@yaacov
Copy link
Member

yaacov commented Oct 25, 2017

When I delete the pod, Hawkular stops giving me samples,

a.
On my setup, it looks like deleted pod, stay in Prometheus and in Hawkular, I will do more tests.

b.
Prometheus and in Hawkular have a short retention time, if you pass that time, all data about pod disappear:
Default retention time of Hawkular is 7 days [1]
Default retention time of Prometheus is 6 hours [2]

[1] https://github.com/openshift/origin-metrics/blob/master/metrics.yaml#L150
[2] https://github.com/openshift/origin/blob/master/examples/prometheus/prometheus.yaml#L145

@Ladas
Copy link
Contributor Author

Ladas commented Oct 25, 2017

@yaacov it was my fault, last commit fixes it, we have to keep container_project_id, which is used on the background in the query. So now it works and collect metrics for disconnected entities. :-)

Copy link
Contributor

@cben cben left a comment

Choose a reason for hiding this comment

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

Very nice!

@@ -20,6 +20,10 @@ def self.historical_start_time
historical_days.days.ago.utc.beginning_of_day
end

def self.targets_archived_from
Settings.performance.targets.archived_for.to_i_with_method.seconds.ago.utc
Copy link
Contributor

Choose a reason for hiding this comment

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

too.long.chain.didnt.follow :)
do you mind splitting .seconds.ago.utc to another statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Use scope instead of the deleting foreign key. Node relation is
required for metrics capture.
Add configurable targets_archived_for, that we use for collecting
metrics.
Collect metrics for archived Container entities. We need this
for shortlived entities (e.g. pods), to make sure the entity
is not discovered and archived before the metrics capture
was planned. The amount of time for looking back for archived
items is configurable and by default is 8h.
Do not nullify container_project_id foreign key, since the
project is also important for metrics collection.
Refactor targets_archived_from to be more readable
@Ladas Ladas force-pushed the allow_to_schedule_disconnected_container_entities_for_perf_capture branch from 8d6f112 to 58f788b Compare October 30, 2017 10:23
@miq-bot
Copy link
Member

miq-bot commented Oct 30, 2017

Checked commits Ladas/manageiq@70f8620~...58f788b with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 0 offenses detected
Everything looks fine. 🏆

Copy link
Member

@yaacov yaacov left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@agrare
Copy link
Member

agrare commented Oct 30, 2017

@Ladas can you put in a PR for kubernetes (and openshift if it would break with this change in) since we know this change breaks some spec tests there? I would like to merge these together.

@Ladas
Copy link
Contributor Author

Ladas commented Oct 30, 2017

@agrare will do, it should be just the 1 reverted commit

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

LGTM, @Ladas I'll merge when the kubernetes PR is posted.

@Ladas
Copy link
Contributor Author

Ladas commented Oct 30, 2017

@agrare agrare merged commit 58f788b into ManageIQ:master Oct 30, 2017
agrare added a commit that referenced this pull request Oct 30, 2017
…ontainer_entities_for_perf_capture

Allow to schedule disconnected container entities for perf capture
@agrare agrare added this to the Sprint 72 Ending Oct 30, 2017 milestone Oct 30, 2017
kbrock added a commit to kbrock/manageiq that referenced this pull request Jul 10, 2023
We use deleted_on to represent a record that is deleted.
We moved over to using archived (deleted_on) from old_container_project_id

see also:
- f6fe251
- ManageIQ#16302
kbrock added a commit to kbrock/manageiq that referenced this pull request Jul 11, 2023
We use deleted_on to represent a record that is deleted.
We moved over to using archived (deleted_on) from old_container_project_id

see also:
- f6fe251
- ManageIQ#16302
kbrock added a commit to kbrock/manageiq that referenced this pull request Jul 11, 2023
We use deleted_on to represent a record that is deleted.
We moved over to using archived (deleted_on) from old_container_project_id
Convert all_container_groups to a regular association

see also:
- f6fe251
- ManageIQ#16302
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants