Large numbers of calls to os.Hostname() when reapplying tags

[jtlisi]

After running a stack trace there was a large number of reads to /proc/sys/kernel/hostname

strace -f -p `pidof snapd
41504 openat(AT_FDCWD, "/proc/sys/kernel/hostname", O_RDONLY|O_CLOEXEC) = 35
.
. for each collected metric
.
41504 openat(AT_FDCWD, "/proc/sys/kernel/hostname", O_RDONLY|O_CLOEXEC) = 35

After running a pstree it was confirmed that the origin was snapd
{snapd}(41504)

The source of all of these calls to os.Hostname() seem to come from this little snippet in control/control.go

go func() {
    for m := range cMetrics {
        // Reapply standard tags after collection as a precaution.  It is common for
        // plugin authors to inadvertently overwrite or not pass along the data
        // passed to CollectMetrics so we will help them out here.
        for i := range m {
            m[i] = addStandardAndWorkflowTags(m[i], allTags)
        }
        metrics = append(metrics, m...)
        wg.Done()
    }
}()

control/metrics.go

func addStandardAndWorkflowTags(m core.Metric, allTags map[string]map[string]string) core.Metric {
    hostname, err := hostnameReader.Hostname()

// hostnameReader, hostnamer created for mocking
func init() {
    hostnameReader = &hostnameReaderType{}
}

type hostnamer interface {
    Hostname() (name string, err error)
}

type hostnameReaderType struct{}

func (h *hostnameReaderType) Hostname() (name string, err error) {
    return os.Hostname()
}

While the performance impact isn't that serious due to the nature of /proc/, I do think its expensive enough to store in a var. Let me know what you think and I put in a PR.

Thanks,

Jacob

[IRCody]

I think it makes sense to reduce the number of calls since the hostname shouldn't be changing super often. Were you thinking about having it called once per collect or just once in per snapd startup?

[jtlisi]

Once per collect seems easiest to me, and the performance impact of one call is basically negligible. What do you think?

[IRCody]

I think that makes sense. You are going to make a pr to do this?

On Friday, September 16, 2016, Jacob Lisi notifications@github.com wrote:

Once per collect seems easiest to me, and the performance impact of one
call is basically negligible. What do you think?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1214 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEvUYAEs0kWpjWMeP955nFL-Ncv7XRTjks5qq0Y7gaJpZM4J_W2Q
.

[jtlisi]

Yes, I'll try and have it done by Monday.