-
Notifications
You must be signed in to change notification settings - Fork 132
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
feat: storage sampler for macos #641
Conversation
@@ -150,6 +150,14 @@ func (ss *Sampler) Disabled() bool { | |||
return ss.Interval() <= config.FREQ_DISABLE_SAMPLING | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In macos partitions don't have IO Counters related data so samples won't. But we can use Storage Sampler IOCounters for IO related information inside System Sampler. This is the reason of exporting these 👇
e25e885
to
e5e6d62
Compare
@@ -1,5 +1,7 @@ | |||
// Copyright 2020 New Relic Corporation. All rights reserved. | |||
// SPDX-License-Identifier: Apache-2.0 | |||
// +build linux windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test was failing on macos. Removed from macos tests to move forward while investigating and maybe involving coreint team.
@@ -57,6 +57,24 @@ jobs: | |||
- name: Running Harvest tests | |||
run: make linux/harvest-tests | |||
|
|||
unit-test-macos: | |||
name: Macos unit tests | |||
runs-on: macos-10.15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
macos-11 giving errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which ones? I only get to see 10.15 errors on the GHA record
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some "unexpected errors" : actions/runner-images#841
Same screen as in the link :(
I saw in the comments that wasn't happening in 10 and didn't have any since I changed it
) | ||
|
||
var ( | ||
SupportedFileSystems = map[string]bool{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing FSs
dc17002
to
2d094dd
Compare
//assert.Equal(t, expected, hip.Data()[0]) | ||
|
||
assert.Equal(t, expected.System, "system") | ||
//assert.Equal(t, expected.Distro, "macOS 11.2.3") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are being read from the host machine, and so they have different values depending on where are being run. As I enabled tests in this PR they are raising now :(
I will improve the comment above 😅
//Commented until fix these test (out of scope in this task)
edit: now I realised that it's not properly fixed, as I assert against expected :carlton: . Updating comment and fix
@@ -561,7 +561,7 @@ type Config struct { | |||
PayloadCompressionLevel int `yaml:"payload_compression_level" envconfig:"payload_compression_level"` | |||
|
|||
// PartitionsTTL Time duration to expire the cached list of storage partitions. | |||
// Default: 60 | |||
// Default: 60s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
|
||
// fullSample returns a sample containing usage data | ||
func fullSample(t *testing.T, samples sample.EventBatch) *storage.Sample { | ||
t.Helper() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
12bb401
to
d7c6c46
Compare
To take into account: