Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Commit

Permalink
Removes the plugin log file created by the legacy client lib
Browse files Browse the repository at this point in the history
A plugins stdout and stderr are already captured by snapd and logged
to snapd's log file.  This commit removes the log file created by the
client lib that captures much of the boiler plate interaction between
control and the plugin including including logging heart beats, which
RPC methods were invoked, etc.

- Removes PluginLogPath from args sent to a plugin
- Adds LogLevel to args sent to a plugin
- Replaces log with logrus in legacy client lib
  • Loading branch information
jcooklin committed Aug 30, 2016
1 parent f08160b commit 3d8056a
Show file tree
Hide file tree
Showing 11 changed files with 48 additions and 69 deletions.
4 changes: 1 addition & 3 deletions control/available_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ func TestAvailablePlugin(t *testing.T) {
Convey("returns nil if plugin successfully stopped", func() {
r := newRunner()
r.SetEmitter(new(MockEmitter))
a := plugin.Arg{
PluginLogPath: "/tmp/snap-test-plugin-stop.log",
}
a := plugin.Arg{}

exPlugin, _ := plugin.NewExecutablePlugin(a, fixtures.PluginPath)
ap, err := r.startPlugin(exPlugin)
Expand Down
2 changes: 1 addition & 1 deletion control/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ type managesPlugins interface {
LoadPlugin(*pluginDetails, gomit.Emitter) (*loadedPlugin, serror.SnapError)
UnloadPlugin(core.Plugin) (*loadedPlugin, serror.SnapError)
SetMetricCatalog(catalogsMetrics)
GenerateArgs(pluginPath string) plugin.Arg
GenerateArgs(logLevel int) plugin.Arg
SetPluginConfig(*pluginConfig)
}

Expand Down
2 changes: 1 addition & 1 deletion control/control_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (m *MockPluginManagerBadSwap) teardown() {}
func (m *MockPluginManagerBadSwap) SetPluginConfig(*pluginConfig) {}
func (m *MockPluginManagerBadSwap) SetMetricCatalog(catalogsMetrics) {}
func (m *MockPluginManagerBadSwap) SetEmitter(gomit.Emitter) {}
func (m *MockPluginManagerBadSwap) GenerateArgs(string) plugin.Arg { return plugin.Arg{} }
func (m *MockPluginManagerBadSwap) GenerateArgs(int) plugin.Arg { return plugin.Arg{} }

func (m *MockPluginManagerBadSwap) all() map[string]*loadedPlugin {
return m.loadedPlugins.table
Expand Down
5 changes: 2 additions & 3 deletions control/plugin/collector_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/intelsdi-x/snap/control/plugin/encoding"
"github.com/intelsdi-x/snap/core"

log "github.com/Sirupsen/logrus"
. "github.com/smartystreets/goconvey/convey"
)

Expand Down Expand Up @@ -98,9 +99,7 @@ func (p *mockErrorPlugin) GetConfigPolicy() (*cpolicy.ConfigPolicy, error) {
func TestCollectorProxy(t *testing.T) {
Convey("Test collector plugin proxy for get metric types ", t, func() {

logger := log.New(os.Stdout,
"test: ",
log.Ldate|log.Ltime|log.Lshortfile)
logger := log.New()
mockPlugin := &mockPlugin{}

mockSessionState := &MockSessionState{
Expand Down
11 changes: 6 additions & 5 deletions control/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"encoding/json"
"fmt"
"io" // Don't use "fmt.Print*"
"log"
"net"
"net/http"
"net/rpc"
Expand All @@ -35,6 +34,8 @@ import (
"runtime"
"time"

log "github.com/Sirupsen/logrus"

"github.com/intelsdi-x/snap/control/plugin/cpolicy"
)

Expand Down Expand Up @@ -225,8 +226,8 @@ func NewPluginMeta(name string, version int, pluginType PluginType, acceptConten

// Arguments passed to startup of Plugin
type Arg struct {
// Plugin file path to binary
PluginLogPath string
// Plugin log level
LogLevel log.Level
// Ping timeout duration
PingTimeoutDuration time.Duration

Expand All @@ -235,9 +236,9 @@ type Arg struct {
listenPort string
}

func NewArg(logpath string) Arg {
func NewArg(logLevel int) Arg {
return Arg{
PluginLogPath: logpath,
LogLevel: log.Level(logLevel),
PingTimeoutDuration: PingTimeoutDurationDefault,
}
}
Expand Down
3 changes: 2 additions & 1 deletion control/plugin/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"testing"
"time"

log "github.com/Sirupsen/logrus"
"github.com/intelsdi-x/snap/core"
. "github.com/smartystreets/goconvey/convey"
)
Expand Down Expand Up @@ -60,7 +61,7 @@ func TestMetricType(t *testing.T) {

func TestArg(t *testing.T) {
Convey("NewArg", t, func() {
arg := NewArg("/tmp/snap/plugin.log")
arg := NewArg(int(log.InfoLevel))
So(arg, ShouldNotBeNil)
})
}
Expand Down
35 changes: 18 additions & 17 deletions control/plugin/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,19 @@ limitations under the License.
package plugin

import (
"bytes"
"crypto/rand"
"crypto/rsa"
"encoding/base64"
"encoding/gob"
"encoding/json"
"errors"
"fmt"
"log"
"os"
"runtime"
"time"

log "github.com/Sirupsen/logrus"

"github.com/intelsdi-x/snap/control/plugin/cpolicy"
"github.com/intelsdi-x/snap/control/plugin/encoding"
"github.com/intelsdi-x/snap/control/plugin/encrypter"
Expand Down Expand Up @@ -256,22 +257,12 @@ func NewSessionState(pluginArgsMsg string, plugin Plugin, meta *PluginMeta) (*Se
rand.Read(rb)
rs := base64.URLEncoding.EncodeToString(rb)

// Initialize a logger based on PluginLogPath
truncOrAppend := os.O_TRUNC // truncate log file explicitly given by user
// Empty or /tmp means use default tmp log (needs to be removed post-aAtruncOrAppendpha)
if pluginArg.PluginLogPath == "" || pluginArg.PluginLogPath == "/tmp" {
if runtime.GOOS == "windows" {
pluginArg.PluginLogPath = `c:\TEMP\snap_plugin.log`
} else {
pluginArg.PluginLogPath = "/tmp/snap_plugin.log"
}
truncOrAppend = os.O_APPEND
}
lf, err := os.OpenFile(pluginArg.PluginLogPath, os.O_WRONLY|os.O_CREATE|truncOrAppend, 0666)
if err != nil {
return nil, errors.New(fmt.Sprintf("error opening log file: %v", err)), 3
logger := &log.Logger{
Out: os.Stderr,
Formatter: &simpleFormatter{},
Hooks: make(log.LevelHooks),
Level: pluginArg.LogLevel,
}
logger := log.New(lf, ">>>", log.Ldate|log.Ltime)

var enc encoding.Encoder
switch meta.RPCType {
Expand Down Expand Up @@ -319,3 +310,13 @@ func init() {
gob.RegisterName("conf_policy_float", &cpolicy.FloatRule{})
gob.RegisterName("conf_policy_bool", &cpolicy.BoolRule{})
}

// simpleFormatter is a logrus formatter that includes only the message.
type simpleFormatter struct{}

func (_ *simpleFormatter) Format(entry *log.Entry) ([]byte, error) {
b := &bytes.Buffer{}
fmt.Fprintf(b, "%s", entry.Message)
b.WriteByte('\n')
return b.Bytes(), nil
}
14 changes: 5 additions & 9 deletions control/plugin/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ package plugin
import (
"encoding/json"
"errors"
"log"
"os"
"testing"
"time"

log "github.com/Sirupsen/logrus"

"github.com/intelsdi-x/snap/control/plugin/cpolicy"
"github.com/intelsdi-x/snap/control/plugin/encoding"
. "github.com/smartystreets/goconvey/convey"
Expand Down Expand Up @@ -127,7 +127,7 @@ func TestSessionState(t *testing.T) {
Arg: &Arg{PingTimeoutDuration: 500 * time.Millisecond},
Encoder: encoding.NewJsonEncoder(),
}
ss.logger = log.New(os.Stdout, ">>>", log.Ldate|log.Ltime)
ss.logger = log.New()
Convey("Ping", func() {

ss.Ping([]byte{}, &[]byte{})
Expand Down Expand Up @@ -202,9 +202,7 @@ func TestSessionState(t *testing.T) {

func TestGetConfigPolicy(t *testing.T) {
Convey("Get Config Policy", t, func() {
logger := log.New(os.Stdout,
"test: ",
log.Ldate|log.Ltime|log.Lshortfile)
logger := log.New()
mockPlugin := &mockPlugin{}

mockSessionState := &MockSessionState{
Expand All @@ -227,9 +225,7 @@ func TestGetConfigPolicy(t *testing.T) {
So(cpr.Policy, ShouldNotBeNil)
})
Convey("Get error in Config Policy ", t, func() {
logger := log.New(os.Stdout,
"test: ",
log.Ldate|log.Ltime|log.Lshortfile)
logger := log.New()
errSession := &errSessionState{
&MockSessionState{
Encoder: encoding.NewGobEncoder(),
Expand Down
7 changes: 3 additions & 4 deletions control/plugin_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func (p *pluginManager) LoadPlugin(details *pluginDetails, emitter gomit.Emitter
"_block": "load-plugin",
"path": filepath.Base(lPlugin.Details.Exec),
}).Info("plugin load called")
ePlugin, err := plugin.NewExecutablePlugin(p.GenerateArgs(lPlugin.Details.Exec), path.Join(lPlugin.Details.ExecPath, lPlugin.Details.Exec))
ePlugin, err := plugin.NewExecutablePlugin(p.GenerateArgs(int(log.GetLevel())), path.Join(lPlugin.Details.ExecPath, lPlugin.Details.Exec))
if err != nil {
pmLogger.WithFields(log.Fields{
"_block": "load-plugin",
Expand Down Expand Up @@ -559,9 +559,8 @@ func (p *pluginManager) UnloadPlugin(pl core.Plugin) (*loadedPlugin, serror.Snap
}

// GenerateArgs generates the cli args to send when stating a plugin
func (p *pluginManager) GenerateArgs(pluginPath string) plugin.Arg {
pluginLog := filepath.Join(p.logPath, filepath.Base(pluginPath)) + ".log"
return plugin.NewArg(pluginLog)
func (p *pluginManager) GenerateArgs(logLevel int) plugin.Arg {
return plugin.NewArg(logLevel)
}

func (p *pluginManager) teardown() {
Expand Down
2 changes: 1 addition & 1 deletion control/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func (r *runner) runPlugin(details *pluginDetails) error {
}
details.ExecPath = path.Join(tempPath, "rootfs")
}
ePlugin, err := plugin.NewExecutablePlugin(r.pluginManager.GenerateArgs(details.Exec), path.Join(details.ExecPath, details.Exec))
ePlugin, err := plugin.NewExecutablePlugin(r.pluginManager.GenerateArgs(int(log.GetLevel())), path.Join(details.ExecPath, details.Exec))
if err != nil {
runnerLog.WithFields(log.Fields{
"_block": "run-plugin",
Expand Down
32 changes: 8 additions & 24 deletions control/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ type MockController struct {

func (p *MockController) GenerateArgs(daemon bool) plugin.Arg {
a := plugin.Arg{
PluginLogPath: "/tmp/snap-test-plugin.log",
NoDaemon: daemon,
NoDaemon: daemon,
}
return a
}
Expand Down Expand Up @@ -353,10 +352,7 @@ func TestRunnerPluginRunning(t *testing.T) {
Convey("should return an AvailablePlugin", func() {
r := newRunner()
r.SetEmitter(new(MockEmitter))
a := plugin.Arg{
PluginLogPath: "/tmp/snap-test-plugin.log",
// Daemon: true,
}
a := plugin.Arg{}
exPlugin, err := newExecutablePlugin(a, fixtures.PluginPath)
if err != nil {
panic(err)
Expand All @@ -378,9 +374,7 @@ func TestRunnerPluginRunning(t *testing.T) {
Convey("availablePlugins should include returned availablePlugin", func() {
r := newRunner()
r.SetEmitter(new(MockEmitter))
a := plugin.Arg{
PluginLogPath: "/tmp/snap-test-plugin.log",
}
a := plugin.Arg{}
exPlugin, err := newExecutablePlugin(a, fixtures.PluginPath)
if err != nil {
panic(err)
Expand All @@ -398,9 +392,7 @@ func TestRunnerPluginRunning(t *testing.T) {
Convey("healthcheck on healthy plugin does not increment failedHealthChecks", func() {
r := newRunner()
r.SetEmitter(new(MockEmitter))
a := plugin.Arg{
PluginLogPath: "/tmp/snap-test-plugin.log",
}
a := plugin.Arg{}
exPlugin, err := newExecutablePlugin(a, fixtures.PluginPath)
if err != nil {
panic(err)
Expand All @@ -417,9 +409,7 @@ func TestRunnerPluginRunning(t *testing.T) {
Convey("healthcheck on unhealthy plugin increments failedHealthChecks", func() {
r := newRunner()
r.SetEmitter(new(MockEmitter))
a := plugin.Arg{
PluginLogPath: "/tmp/snap-test-plugin.log",
}
a := plugin.Arg{}
exPlugin, err := newExecutablePlugin(a, fixtures.PluginPath)
if err != nil {
panic(err)
Expand All @@ -436,9 +426,7 @@ func TestRunnerPluginRunning(t *testing.T) {
Convey("successful healthcheck resets failedHealthChecks", func() {
r := newRunner()
r.SetEmitter(new(MockEmitter))
a := plugin.Arg{
PluginLogPath: "/tmp/snap-test-plugin-foo.log",
}
a := plugin.Arg{}
exPlugin, err := newExecutablePlugin(a, fixtures.PluginPath)
if err != nil {
panic(err)
Expand All @@ -459,9 +447,7 @@ func TestRunnerPluginRunning(t *testing.T) {
Convey("three consecutive failedHealthChecks disables the plugin", func() {
r := newRunner()
r.SetEmitter(new(MockEmitter))
a := plugin.Arg{
PluginLogPath: "/tmp/snap-test-plugin.log",
}
a := plugin.Arg{}
exPlugin, err := newExecutablePlugin(a, fixtures.PluginPath)
if err != nil {
panic(err)
Expand Down Expand Up @@ -494,9 +480,7 @@ func TestRunnerPluginRunning(t *testing.T) {
Convey("should return an AvailablePlugin in a Running state", func() {
r := newRunner()
r.SetEmitter(new(MockEmitter))
a := plugin.Arg{
PluginLogPath: "/tmp/snap-test-plugin-stop.log",
}
a := plugin.Arg{}
exPlugin, err := newExecutablePlugin(a, fixtures.PluginPath)
if err != nil {
panic(err)
Expand Down

0 comments on commit 3d8056a

Please sign in to comment.