Skip to content

Commit

Permalink
[7.x] Fix minor diff to master (#3546)
Browse files Browse the repository at this point in the history
* Remove non-existing config option

* Tmp validation fix for self instrumentation cfg

Validation `nonzero` is broken with latest beats update. Temporarily
implement validation instead of using validation tags until
elastic/go-ucfg#147 is fixed.

backports commit f604f27
  • Loading branch information
simitt authored Mar 24, 2020
1 parent c73c700 commit 6996b79
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 5 deletions.
1 change: 0 additions & 1 deletion beater/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@ func Test_UnpackConfig(t *testing.T) {
},
},
},
"jaeger.enabled": true,
"jaeger.grpc.enabled": true,
"api_key.enabled": true,
},
Expand Down
12 changes: 11 additions & 1 deletion beater/config/instrumentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"github.com/elastic/beats/v7/libbeat/logp"
"github.com/elastic/go-ucfg"
)

const (
Expand All @@ -33,12 +34,21 @@ const (
type InstrumentationConfig struct {
Enabled *bool `config:"enabled"`
Environment *string `config:"environment"`
Hosts urls `config:"hosts" validate:"nonzero"`
Hosts urls `config:"hosts"` //TODO(simi): add `validate:"nonzero"` again once https://github.com/elastic/go-ucfg/issues/147 is fixed
Profiling ProfilingConfig `config:"profiling"`
APIKey string `config:"api_key"`
SecretToken string `config:"secret_token"`
}

func (c *InstrumentationConfig) Validate() error {
for _, h := range c.Hosts {
if h == nil || h.Host == "" {
return ucfg.ErrZeroValue
}
}
return nil
}

// IsEnabled indicates whether self instrumentation is enabled
func (c *InstrumentationConfig) IsEnabled() bool {
// self instrumentation is disabled by default.
Expand Down
48 changes: 48 additions & 0 deletions beater/config/instrumentation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Licensed to Elasticsearch B.V. under one or more contributor
// license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright
// ownership. Elasticsearch B.V. licenses this file to you under
// the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

package config

import (
"testing"

"github.com/elastic/go-ucfg"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/elastic/beats/v7/libbeat/common"
)

func TestNonzeroHosts(t *testing.T) {
t.Run("ZeroHost", func(t *testing.T) {
cfg, err := NewConfig("9.9.9",
common.MustNewConfigFrom(map[string]interface{}{
"instrumentation.enabled": true, "instrumentation.hosts": []string{""}}), nil)
require.Error(t, err)
assert.Contains(t, err.Error(), ucfg.ErrZeroValue.Error())
assert.Nil(t, cfg)
})

t.Run("Valid", func(t *testing.T) {
cfg, err := NewConfig("9.9.9",
common.MustNewConfigFrom(map[string]interface{}{"instrumentation.enabled": true}), nil)
require.NoError(t, err)
assert.True(t, *cfg.SelfInstrumentation.Enabled)
assert.Empty(t, cfg.SelfInstrumentation.Hosts)
})
}
5 changes: 2 additions & 3 deletions beater/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (s server) run(listener net.Listener, tracerServer *tracerServer) error {
g.Go(func() error {
return s.httpServer.start(listener)
})
if s.isAvailable() {
if s.isAvailable(s.cfg.ShutdownTimeout) {
notifyListening(context.Background(), s.cfg, s.reporter)
}
if tracerServer != nil {
Expand All @@ -98,12 +98,11 @@ func (s server) stop() {
}
}

func (s server) isAvailable() bool {
func (s server) isAvailable(timeout time.Duration) bool {
// following an example from https://golang.org/pkg/net/
// dial into tcp connection to ensure listener is ready, send get request and read response,
// in case tls is enabled, the server will respond with 400,
// as this only checks the server is up and reachable errors can be ignored
timeout := s.cfg.ShutdownTimeout
conn, err := net.DialTimeout("tcp", s.cfg.Host, timeout)
if err != nil {
return false
Expand Down

0 comments on commit 6996b79

Please sign in to comment.