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

cmd/scollector: Covert gauges to counters for os.cpu #1494

Merged
merged 1 commit into from
Dec 3, 2015
Merged

Conversation

kylebrandt
Copy link
Member

No description provided.

type tsIntegrator func(int64, float64) float64

func getTsIntegrator() tsIntegrator {
var i float64
Copy link
Contributor

Choose a reason for hiding this comment

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

these variable names are not descriptive enough to indicate what is going on here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

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

i,ts,t,v -> total,lastTimestamp, time, value

Copy link
Member Author

Choose a reason for hiding this comment

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

oldTs and ts? i to total?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

How about

type tsIntegrator struct{
   data map[string]struct{
      total floats64
      lastTime
   }
   sync.Mutex
}

?

then you can do cpuIntegrator.Add(key, now, val) and cpuIntegrator.Total(key). Hide all of the mutexing and map stuff inside the integrator.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking there might be use cases where you don't want to use it in
the map, so this would make it more reusable

On Thu, Dec 3, 2015 at 10:23 AM, Craig Peterson notifications@github.com
wrote:

In cmd/scollector/collectors/collectors.go
#1494 (comment):

@@ -325,3 +325,17 @@ func skipMetric(index string) bool {
}
return true
}
+
+type tsIntegrator func(int64, float64) float64
+
+func getTsIntegrator() tsIntegrator {

  • var i float64

How about

type tsIntegrator struct{
data map[string]struct{
total floats64
lastTime
}
sync.Mutex
}

?

then you can do cpuIntegrator.Add(key, now, val) and
cpuIntegrator.Total(key). Hide all of the mutexing and map stuff inside
the integrator.


Reply to this email directly or view it on GitHub
https://github.com/bosun-monitor/bosun/pull/1494/files#r46564464.

Interval: time.Second * 30,
name: fmt.Sprintf("snmp-cisco-%s", cfg.Host),
})
&IntervalCollector{
Copy link
Contributor

Choose a reason for hiding this comment

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

if you make the integrator here and pass it into c_cisco_cpu, you may not need the map or mutex at all.

@kylebrandt kylebrandt force-pushed the intCpu branch 2 times, most recently from f98348d to 5da18a0 Compare December 3, 2015 15:47
@kylebrandt kylebrandt merged commit f4d9667 into master Dec 3, 2015
@gbrayut gbrayut deleted the intCpu branch December 30, 2015 01:38
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.

2 participants