Skip to content

Commit

Permalink
Merge pull request #559 from spiral/log_does_not_show_any_message
Browse files Browse the repository at this point in the history
fix(various): Small fixes before release
  • Loading branch information
rustatian authored Feb 23, 2021
2 parents 123c5e9 + fdbf6a6 commit 18a0972
Show file tree
Hide file tree
Showing 15 changed files with 44 additions and 102 deletions.
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/feature_request.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: Feature request
about: Suggest an idea for this project
title: "[FEATURE REQUEST]"
labels: C-feature-request
assignees: 48d90782, wolfy-j
assignees: 48d90782

---

Expand Down
2 changes: 1 addition & 1 deletion .rr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ http:
supervisor:
# watch_tick defines how often to check the state of the workers (seconds)
watch_tick: 1s
# ttl defines maximum time worker is allowed to live (seconds)
# ttl defines maximum time worker is allowed to live (seconds) (soft)
ttl: 0
# idle_ttl defines maximum duration worker can spend in idle mode after first use. Disabled when 0 (seconds)
idle_ttl: 10s
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ v2.0.0-RC.4 (20.02.2021)

- PHP tests use latest signatures (https://github.com/spiral/roadrunner/pull/550)
- Endure container update to v1.0.0-RC.2 version
- Remove unneeded mutex from the `http.Workers` method.
- Rename `checker` plugin package to `status`, remove `/v1` endpoint prefix (#557)
- Add static, headers, status, gzip plugins to the `main.go`.

v2.0.0-RC.3 (17.02.2021)
-------------------
Expand Down
8 changes: 4 additions & 4 deletions plugins/gzip/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,19 @@ import (

const PluginName = "gzip"

type Gzip struct{}
type Plugin struct{}

// needed for the Endure
func (g *Gzip) Init() error {
func (g *Plugin) Init() error {
return nil
}

func (g *Gzip) Middleware(next http.Handler) http.HandlerFunc {
func (g *Plugin) Middleware(next http.Handler) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
gziphandler.GzipHandler(next).ServeHTTP(w, r)
}
}

func (g *Gzip) Name() string {
func (g *Plugin) Name() string {
return PluginName
}
17 changes: 6 additions & 11 deletions plugins/http/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ import (
"github.com/spiral/errors"
"github.com/spiral/roadrunner/v2/pkg/pool"
"github.com/spiral/roadrunner/v2/pkg/worker"
"github.com/spiral/roadrunner/v2/plugins/checker"
"github.com/spiral/roadrunner/v2/plugins/config"
"github.com/spiral/roadrunner/v2/plugins/http/attributes"
httpConfig "github.com/spiral/roadrunner/v2/plugins/http/config"
"github.com/spiral/roadrunner/v2/plugins/logger"
"github.com/spiral/roadrunner/v2/plugins/server"
"github.com/spiral/roadrunner/v2/plugins/status"
"github.com/spiral/roadrunner/v2/utils"
"golang.org/x/net/http2"
"golang.org/x/net/http2/h2c"
Expand Down Expand Up @@ -109,12 +109,9 @@ func (s *Plugin) Init(cfg config.Configurer, log logger.Logger, server server.Se

func (s *Plugin) logCallback(event interface{}) {
if ev, ok := event.(ResponseEvent); ok {
s.log.Debug("",
s.log.Debug(fmt.Sprintf("%d %s %s", ev.Response.Status, ev.Request.Method, ev.Request.URI),
"remote", ev.Request.RemoteAddr,
"ts", ev.Elapsed().String(),
"resp.status", ev.Response.Status,
"method", ev.Request.Method,
"uri", ev.Request.URI,
"elapsed", ev.Elapsed().String(),
)
}
}
Expand Down Expand Up @@ -303,8 +300,6 @@ func (s *Plugin) ServeHTTP(w http.ResponseWriter, r *http.Request) {

// Workers returns associated pool workers
func (s *Plugin) Workers() []worker.BaseProcess {
s.Lock()
defer s.Unlock()
return s.pool.Workers()
}

Expand Down Expand Up @@ -365,17 +360,17 @@ func (s *Plugin) AddMiddleware(name endure.Named, m Middleware) {
}

// Status return status of the particular plugin
func (s *Plugin) Status() checker.Status {
func (s *Plugin) Status() status.Status {
workers := s.Workers()
for i := 0; i < len(workers); i++ {
if workers[i].State().IsActive() {
return checker.Status{
return status.Status{
Code: http.StatusOK,
}
}
}
// if there are no workers, threat this as error
return checker.Status{
return status.Status{
Code: http.StatusInternalServerError,
}
}
Expand Down
3 changes: 1 addition & 2 deletions plugins/static/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (s *Plugin) Init(cfg config.Configurer, log logger.Logger) error {

err = s.cfg.Valid()
if err != nil {
return errors.E(op, errors.Disabled, err)
return errors.E(op, err)
}

return nil
Expand Down Expand Up @@ -88,7 +88,6 @@ func (s *Plugin) handleStatic(w http.ResponseWriter, r *http.Request) bool {

f, err := s.root.Open(fPath)
if err != nil {
s.log.Error("file open error", "error", err)
if s.cfg.AlwaysServe(fPath) {
w.WriteHeader(404)
return true
Expand Down
2 changes: 1 addition & 1 deletion plugins/checker/config.go → plugins/status/config.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package checker
package status

type Config struct {
Address string
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package checker
package status

// Status consists of status code from the service
type Status struct {
Expand Down
15 changes: 7 additions & 8 deletions plugins/checker/plugin.go → plugins/status/plugin.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
package checker
package status

import (
"fmt"
"net/http"
"time"

"github.com/gofiber/fiber/v2"
fiberLogger "github.com/gofiber/fiber/v2/middleware/logger"
endure "github.com/spiral/endure/pkg/container"
"github.com/spiral/errors"
"github.com/spiral/roadrunner/v2/plugins/config"
Expand Down Expand Up @@ -43,12 +42,12 @@ func (c *Plugin) Init(log logger.Logger, cfg config.Configurer) error {
func (c *Plugin) Serve() chan error {
errCh := make(chan error, 1)
c.server = fiber.New(fiber.Config{
ReadTimeout: time.Second * 5,
WriteTimeout: time.Second * 5,
IdleTimeout: time.Second * 5,
ReadTimeout: time.Second * 5,
WriteTimeout: time.Second * 5,
IdleTimeout: time.Second * 5,
DisableStartupMessage: true,
})
c.server.Group("/v1", c.healthHandler)
c.server.Use(fiberLogger.New())

c.server.Use("/health", c.healthHandler)

go func() {
Expand Down Expand Up @@ -120,7 +119,7 @@ func (c *Plugin) healthHandler(ctx *fiber.Ctx) error {

if len(plugins.Plugins) == 0 {
ctx.Status(http.StatusOK)
_, _ = ctx.WriteString("No plugins provided in query. Query should be in form of: /v1/health?plugin=plugin1&plugin=plugin2 \n")
_, _ = ctx.WriteString("No plugins provided in query. Query should be in form of: health?plugin=plugin1&plugin=plugin2 \n")
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion plugins/checker/rpc.go → plugins/status/rpc.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package checker
package status

import (
"github.com/spiral/errors"
Expand Down
3 changes: 1 addition & 2 deletions tests/plugins/checker/configs/.rr-checker-init.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ server:
command: "php ../../http/client.php echo pipes"
user: ""
group: ""
env:
"RR_HTTP": "true"
relay: "pipes"
relay_timeout: "20s"

status:
address: "127.0.0.1:34333"

logs:
mode: development
level: error
Expand Down
10 changes: 5 additions & 5 deletions tests/plugins/checker/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import (

endure "github.com/spiral/endure/pkg/container"
goridgeRpc "github.com/spiral/goridge/v3/pkg/rpc"
"github.com/spiral/roadrunner/v2/plugins/checker"
"github.com/spiral/roadrunner/v2/plugins/config"
httpPlugin "github.com/spiral/roadrunner/v2/plugins/http"
"github.com/spiral/roadrunner/v2/plugins/logger"
rpcPlugin "github.com/spiral/roadrunner/v2/plugins/rpc"
"github.com/spiral/roadrunner/v2/plugins/server"
"github.com/spiral/roadrunner/v2/plugins/status"
"github.com/stretchr/testify/assert"
)

Expand All @@ -37,7 +37,7 @@ func TestStatusHttp(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&checker.Plugin{},
&status.Plugin{},
)
assert.NoError(t, err)

Expand Down Expand Up @@ -95,7 +95,7 @@ const resp = `Service: http: Status: 200
Service: rpc not found`

func checkHTTPStatus(t *testing.T) {
req, err := http.NewRequest("GET", "http://127.0.0.1:34333/v1/health?plugin=http&plugin=rpc", nil)
req, err := http.NewRequest("GET", "http://127.0.0.1:34333/health?plugin=http&plugin=rpc", nil)
assert.NoError(t, err)

r, err := http.DefaultClient.Do(req)
Expand Down Expand Up @@ -124,7 +124,7 @@ func TestStatusRPC(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&checker.Plugin{},
&status.Plugin{},
)
assert.NoError(t, err)

Expand Down Expand Up @@ -182,7 +182,7 @@ func checkRPCStatus(t *testing.T) {
assert.NoError(t, err)
client := rpc.NewClientWithCodec(goridgeRpc.NewClientCodec(conn))

st := &checker.Status{}
st := &status.Status{}

err = client.Call("status.Status", "http", &st)
assert.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions tests/plugins/gzip/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestGzipPlugin(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
)
assert.NoError(t, err)

Expand Down Expand Up @@ -126,7 +126,7 @@ func TestMiddlewareNotExist(t *testing.T) {
mockLogger,
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
)
assert.NoError(t, err)

Expand Down
2 changes: 1 addition & 1 deletion tests/plugins/http/http_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ logs:
mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes()
mockLogger.EXPECT().Debug("worker destructed", "pid", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("worker constructed", "pid", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("", "remote", gomock.Any(), "ts", gomock.Any(), "resp.status", gomock.Any(), "method", gomock.Any(), "uri", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("201 GET http://localhost:34999/?hello=world", "remote", "127.0.0.1", "elapsed", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("WORLD", "pid", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("worker event received", "event", events.EventWorkerLog, "worker state", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() // placeholder for the workerlogerror
Expand Down
71 changes: 9 additions & 62 deletions tests/plugins/static/static_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestStaticPlugin(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
&static.Plugin{},
)
assert.NoError(t, err)
Expand Down Expand Up @@ -138,7 +138,7 @@ func serveStaticSample(t *testing.T) {
_ = r.Body.Close()
}

func TestStaticDisabled(t *testing.T) {
func TestStaticDisabled_Error(t *testing.T) {
cont, err := endure.NewContainer(nil, endure.SetLogLevel(endure.ErrorLevel))
assert.NoError(t, err)

Expand All @@ -152,66 +152,11 @@ func TestStaticDisabled(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
&static.Plugin{},
)
assert.NoError(t, err)

err = cont.Init()
if err != nil {
t.Fatal(err)
}

ch, err := cont.Serve()
assert.NoError(t, err)

sig := make(chan os.Signal, 1)
signal.Notify(sig, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)

wg := &sync.WaitGroup{}
wg.Add(1)

stopCh := make(chan struct{}, 1)

go func() {
defer wg.Done()
for {
select {
case e := <-ch:
assert.Fail(t, "error", e.Error.Error())
err = cont.Stop()
if err != nil {
assert.FailNow(t, "error", err.Error())
}
case <-sig:
err = cont.Stop()
if err != nil {
assert.FailNow(t, "error", err.Error())
}
return
case <-stopCh:
// timeout
err = cont.Stop()
if err != nil {
assert.FailNow(t, "error", err.Error())
}
return
}
}
}()

time.Sleep(time.Second)
t.Run("StaticDisabled", staticDisabled)

stopCh <- struct{}{}
wg.Wait()
}

func staticDisabled(t *testing.T) {
_, r, err := get("http://localhost:21234/sample.txt") //nolint:bodyclose
assert.NoError(t, err)
assert.NotNil(t, r)
assert.Empty(t, r.Header.Get("X-Powered-By"))
assert.Error(t, cont.Init())
}

func TestStaticFilesDisabled(t *testing.T) {
Expand All @@ -228,7 +173,7 @@ func TestStaticFilesDisabled(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
&static.Plugin{},
)
assert.NoError(t, err)
Expand Down Expand Up @@ -306,7 +251,9 @@ func TestStaticFilesForbid(t *testing.T) {

mockLogger.EXPECT().Debug("worker destructed", "pid", gomock.Any()).AnyTimes()
mockLogger.EXPECT().Debug("worker constructed", "pid", gomock.Any()).AnyTimes()
mockLogger.EXPECT().Debug("", "remote", gomock.Any(), "ts", gomock.Any(), "resp.status", gomock.Any(), "method", gomock.Any(), "uri", gomock.Any()).AnyTimes()
mockLogger.EXPECT().Debug("201 GET http://localhost:34653/http?hello=world", "remote", "127.0.0.1", "elapsed", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("201 GET http://localhost:34653/client.XXX?hello=world", "remote", "127.0.0.1", "elapsed", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("201 GET http://localhost:34653/client.php?hello=world", "remote", "127.0.0.1", "elapsed", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Error("file open error", "error", gomock.Any()).AnyTimes()
mockLogger.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() // placeholder for the workerlogerror

Expand All @@ -315,7 +262,7 @@ func TestStaticFilesForbid(t *testing.T) {
mockLogger,
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
&static.Plugin{},
)
assert.NoError(t, err)
Expand Down

0 comments on commit 18a0972

Please sign in to comment.