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

Attempt to handle "kstat-based metrics produce samples from the 1980's" #6589

Merged
merged 2 commits into from
Sep 18, 2024

Conversation

zeeshanlakhani
Copy link
Collaborator

Closes #5899.

Instead of checking for dates or anything based on the data sampled, this update starts collecting kstat samples (i.e. being interested in them) once the sled agent is synchronized with NTP.

We leverage the metrics manager associated with the agent to now look for a new message and update the tracked links we've added/tracked thus far.

Closes #5899.

Instead of checking for dates or anything based on the data sampled, this
update starts collecting kstat samples (i.e. being interested in them) once
the sled agent is synchronized with NTP.

We leverage the metrics manager associated with the agent to now look
for a new message and update the tracked links we've added/tracked thus
far.
@zeeshanlakhani
Copy link
Collaborator Author

@bnaecker is definitely a different path than what you were looking at in the issue, but with the addition of the metrics manager, this seemed to be more robust, and it doesn't involve interval pooling or looking at specific ranges (of years/dates).

Copy link
Collaborator

@bnaecker bnaecker left a comment

Choose a reason for hiding this comment

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

Nice, this looks pretty good! I've a few comments mostly around improving documentation and making the invariants a bit more clear. Let me know if something doesn't make sense. Thanks!

oximeter/instruments/src/kstat/link.rs Outdated Show resolved Hide resolved
oximeter/instruments/src/kstat/link.rs Outdated Show resolved Hide resolved
oximeter/instruments/src/kstat/link.rs Outdated Show resolved Hide resolved
sled-agent/src/services.rs Outdated Show resolved Hide resolved
sled-agent/src/services.rs Outdated Show resolved Hide resolved
sled-agent/src/metrics.rs Outdated Show resolved Hide resolved
sled-agent/src/metrics.rs Outdated Show resolved Hide resolved
oximeter/instruments/src/kstat/link.rs Outdated Show resolved Hide resolved
sled-agent/src/services.rs Outdated Show resolved Hide resolved
sled-agent/src/services.rs Outdated Show resolved Hide resolved
@zeeshanlakhani
Copy link
Collaborator Author

zeeshanlakhani commented Sep 18, 2024

Nice, this looks pretty good! I've a few comments mostly around improving documentation and making the invariants a bit more clear. Let me know if something doesn't make sense. Thanks!

Yep. Easy clean-ups. I'll update those this morning.

Updated.

Copy link
Collaborator

@bnaecker bnaecker left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@zeeshanlakhani zeeshanlakhani merged commit 18b8ab3 into main Sep 18, 2024
16 checks passed
@zeeshanlakhani zeeshanlakhani deleted the zl/wait-for-sync-to-sample branch September 18, 2024 16:31
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.

kstat-based metrics produce samples from the 1980's
2 participants