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

Enable Lint Rule: line-length-limit #5540

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
4 changes: 0 additions & 4 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,6 @@ linters-settings:
enable-all-rules: true
# See https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md
rules:
# not a completely bad linter, but needs clean-up and sensible width (80 is too small)
- name: line-length-limit
disabled: true
arguments: [80]
# would be ok if we could exclude the test files, but otherwise too noisy
- name: add-constant
disabled: true
Expand Down
10 changes: 8 additions & 2 deletions cmd/agent/app/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ func (a *Agent) Run() error {
a.httpAddr.Store(listener.Addr().String())
a.exitWG.Add(1)
go func() {
a.logger.Info("Starting jaeger-agent HTTP server", zap.Int("http-port", listener.Addr().(*net.TCPAddr).Port))
a.logger.Info(
"Starting jaeger-agent HTTP server",
zap.Int("http-port", listener.Addr().(*net.TCPAddr).Port),
)
if err := a.httpServer.Serve(listener); err != http.ErrServerClosed {
a.logger.Error("http server failure", zap.Error(err))
}
Expand All @@ -90,7 +93,10 @@ func (a *Agent) HTTPAddr() string {
// Stop forces all agent go routines to exit.
func (a *Agent) Stop() {
// first, close the http server, so that we don't have any more inflight requests
a.logger.Info("shutting down agent's HTTP server", zap.String("addr", a.HTTPAddr()))
a.logger.Info(
"shutting down agent's HTTP server",
zap.String("addr", a.HTTPAddr()),
)
timeout, cancel := context.WithTimeout(context.Background(), 5*time.Second)
if err := a.httpServer.Shutdown(timeout); err != nil {
a.logger.Error("failed to close HTTP server", zap.Error(err))
Expand Down
32 changes: 26 additions & 6 deletions cmd/agent/app/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ import (

func TestAgentStartError(t *testing.T) {
cfg := &Builder{}
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metrics.NullFactory)
agent, err := cfg.CreateAgent(
fakeCollectorProxy{},
zap.NewNop(),
metrics.NullFactory,
)
require.NoError(t, err)
agent.httpServer.Addr = "bad-address"
require.Error(t, agent.Run())
Expand Down Expand Up @@ -100,13 +104,19 @@ func withRunningAgent(t *testing.T, testcase func(string, chan error)) {
}

logger, logBuf := testutils.NewLogger()
mBldr := &metricsbuilder.Builder{HTTPRoute: "/metrics", Backend: "prometheus"}
mBldr := &metricsbuilder.Builder{
HTTPRoute: "/metrics",
Backend: "prometheus",
}
metricsFactory, err := mBldr.CreateMetricsFactory("jaeger")
require.NoError(t, err)
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, logger, metricsFactory)
require.NoError(t, err)
if h := mBldr.Handler(); metricsFactory != nil && h != nil {
logger.Info("Registering metrics handler with HTTP server", zap.String("route", mBldr.HTTPRoute))
logger.Info(
"Registering metrics handler with HTTP server",
zap.String("route", mBldr.HTTPRoute),
)
agent.GetHTTPRouter().Handle(mBldr.HTTPRoute, h).Methods(http.MethodGet)
}
ch := make(chan error, 2)
Expand Down Expand Up @@ -154,7 +164,10 @@ func TestStartStopRace(t *testing.T) {
},
}
logger, logBuf := testutils.NewEchoLogger(t)
mBldr := &metricsbuilder.Builder{HTTPRoute: "/metrics", Backend: "prometheus"}
mBldr := &metricsbuilder.Builder{
HTTPRoute: "/metrics",
Backend: "prometheus",
}
metricsFactory, err := mBldr.CreateMetricsFactory("jaeger")
require.NoError(t, err)
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, logger, metricsFactory)
Expand Down Expand Up @@ -197,10 +210,17 @@ func TestStartStopWithSocketBufferSet(t *testing.T) {
},
},
}
mBldr := &metricsbuilder.Builder{HTTPRoute: "/metrics", Backend: "prometheus"}
mBldr := &metricsbuilder.Builder{
HTTPRoute: "/metrics",
Backend: "prometheus",
}
metricsFactory, err := mBldr.CreateMetricsFactory("jaeger")
require.NoError(t, err)
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metricsFactory)
agent, err := cfg.CreateAgent(
fakeCollectorProxy{},
zap.NewNop(),
metricsFactory,
)
require.NoError(t, err)

if err := agent.Run(); err != nil {
Expand Down
96 changes: 76 additions & 20 deletions cmd/agent/app/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,12 @@ type Model string
type Protocol string

var protocolFactoryMap = map[Protocol]thrift.TProtocolFactory{
compactProtocol: thrift.NewTCompactProtocolFactoryConf(&thrift.TConfiguration{}),
binaryProtocol: thrift.NewTBinaryProtocolFactoryConf(&thrift.TConfiguration{}),
compactProtocol: thrift.NewTCompactProtocolFactoryConf(
&thrift.TConfiguration{},
),
binaryProtocol: thrift.NewTBinaryProtocolFactoryConf(
&thrift.TConfiguration{},
),
}

// CollectorProxy provides access to Reporter and ClientConfigManager
Expand Down Expand Up @@ -90,7 +94,7 @@ type ServerConfiguration struct {
QueueSize int `yaml:"queueSize"`
MaxPacketSize int `yaml:"maxPacketSize"`
SocketBufferSize int `yaml:"socketBufferSize"`
HostPort string `yaml:"hostPort" validate:"nonzero"`
HostPort string `yaml:"hostPort" validate:"nonzero"`
}

// HTTPServerConfiguration holds config for a server providing sampling strategies and baggage restrictions to clients
Expand All @@ -105,13 +109,21 @@ func (b *Builder) WithReporter(r ...reporter.Reporter) *Builder {
}

// CreateAgent creates the Agent
func (b *Builder) CreateAgent(primaryProxy CollectorProxy, logger *zap.Logger, mFactory metrics.Factory) (*Agent, error) {
func (b *Builder) CreateAgent(
primaryProxy CollectorProxy,
logger *zap.Logger,
mFactory metrics.Factory,
) (*Agent, error) {
r := b.getReporter(primaryProxy)
processors, err := b.getProcessors(r, mFactory, logger)
if err != nil {
return nil, fmt.Errorf("cannot create processors: %w", err)
}
server := b.HTTPServer.getHTTPServer(primaryProxy.GetManager(), mFactory, logger)
server := b.HTTPServer.getHTTPServer(
primaryProxy.GetManager(),
mFactory,
logger,
)
b.publishOpts()

return NewAgent(processors, server, logger), nil
Expand All @@ -132,31 +144,54 @@ func (b *Builder) getReporter(primaryProxy CollectorProxy) reporter.Reporter {
func (b *Builder) publishOpts() {
for _, p := range b.Processors {
prefix := fmt.Sprintf(processorPrefixFmt, p.Model, p.Protocol)
safeexpvar.SetInt(prefix+suffixServerMaxPacketSize, int64(p.Server.MaxPacketSize))
safeexpvar.SetInt(prefix+suffixServerQueueSize, int64(p.Server.QueueSize))
safeexpvar.SetInt(
prefix+suffixServerMaxPacketSize,
int64(p.Server.MaxPacketSize),
)
safeexpvar.SetInt(
prefix+suffixServerQueueSize,
int64(p.Server.QueueSize),
)
safeexpvar.SetInt(prefix+suffixWorkers, int64(p.Workers))
}
}

func (b *Builder) getProcessors(rep reporter.Reporter, mFactory metrics.Factory, logger *zap.Logger) ([]processors.Processor, error) {
func (b *Builder) getProcessors(
rep reporter.Reporter,
mFactory metrics.Factory,
logger *zap.Logger,
) ([]processors.Processor, error) {
retMe := make([]processors.Processor, len(b.Processors))
for idx, cfg := range b.Processors {
protoFactory, ok := protocolFactoryMap[cfg.Protocol]
if !ok {
return nil, fmt.Errorf("cannot find protocol factory for protocol %v", cfg.Protocol)
return nil, fmt.Errorf(
"cannot find protocol factory for protocol %v",
cfg.Protocol,
)
}
var handler processors.AgentProcessor
switch cfg.Model {
case jaegerModel, zipkinModel:
handler = agentThrift.NewAgentProcessor(rep)
default:
return nil, fmt.Errorf("cannot find agent processor for data model %v", cfg.Model)
return nil, fmt.Errorf(
"cannot find agent processor for data model %v",
cfg.Model,
)
}
metrics := mFactory.Namespace(metrics.NSOptions{Name: "", Tags: map[string]string{
"protocol": string(cfg.Protocol),
"model": string(cfg.Model),
}})
processor, err := cfg.GetThriftProcessor(metrics, protoFactory, handler, logger)
metrics := mFactory.Namespace(
metrics.NSOptions{Name: "", Tags: map[string]string{
"protocol": string(cfg.Protocol),
"model": string(cfg.Model),
}},
)
processor, err := cfg.GetThriftProcessor(
metrics,
protoFactory,
handler,
logger,
)
if err != nil {
return nil, fmt.Errorf("cannot create Thrift Processor: %w", err)
}
Expand All @@ -166,7 +201,11 @@ func (b *Builder) getProcessors(rep reporter.Reporter, mFactory metrics.Factory,
}

// GetHTTPServer creates an HTTP server that provides sampling strategies and baggage restrictions to client libraries.
func (c HTTPServerConfiguration) getHTTPServer(manager configmanager.ClientConfigManager, mFactory metrics.Factory, logger *zap.Logger) *http.Server {
func (c HTTPServerConfiguration) getHTTPServer(
manager configmanager.ClientConfigManager,
mFactory metrics.Factory,
logger *zap.Logger,
) *http.Server {
hostPort := c.HostPort
if hostPort == "" {
hostPort = defaultHTTPServerHostPort
Expand All @@ -188,7 +227,14 @@ func (c *ProcessorConfiguration) GetThriftProcessor(
return nil, fmt.Errorf("cannot create UDP Server: %w", err)
}

return processors.NewThriftProcessor(server, c.Workers, mFactory, factory, handler, logger)
return processors.NewThriftProcessor(
server,
c.Workers,
mFactory,
factory,
handler,
logger,
)
}

func (c *ProcessorConfiguration) applyDefaults() {
Expand All @@ -202,7 +248,9 @@ func (c *ServerConfiguration) applyDefaults() {
}

// getUDPServer gets a TBufferedServer backed server using the server configuration
func (c *ServerConfiguration) getUDPServer(mFactory metrics.Factory) (servers.Server, error) {
func (c *ServerConfiguration) getUDPServer(
mFactory metrics.Factory,
) (servers.Server, error) {
c.applyDefaults()

if c.HostPort == "" {
Expand All @@ -218,7 +266,12 @@ func (c *ServerConfiguration) getUDPServer(mFactory metrics.Factory) (servers.Se
}
}

return servers.NewTBufferedServer(transport, c.QueueSize, c.MaxPacketSize, mFactory)
return servers.NewTBufferedServer(
transport,
c.QueueSize,
c.MaxPacketSize,
mFactory,
)
}

func defaultInt(value int, defaultVal int) int {
Expand Down Expand Up @@ -246,7 +299,10 @@ func CreateCollectorProxy(
) (CollectorProxy, error) {
builder, ok := builders[opts.ReporterType]
if !ok {
return nil, fmt.Errorf("unknown reporter type %s", string(opts.ReporterType))
return nil, fmt.Errorf(
"unknown reporter type %s",
string(opts.ReporterType),
)
}
return builder(ctx, opts)
}
Loading
Loading