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

Syslog input as wrapper #376

Merged
merged 5 commits into from
Feb 15, 2022

Conversation

djaglowski
Copy link
Member

@djaglowski djaglowski commented Feb 10, 2022

The syslog_input operator was previously implemented
as a builder that built two operators. While this is
a perfectly fine solution, it happens to be the only
place in this codebase where multiple operators are
returned from a builder, excepting plugins.

This new implementation builds the same two operators,
but then wraps them together within a single operator.
This will allow for the eventual simplification of
other code, once plugins are also removed from this
codebase.

Related to #375, #380

@codecov
Copy link

codecov bot commented Feb 10, 2022

Codecov Report

Merging #376 (3407c28) into main (3f71d1b) will increase coverage by 0.1%.
The diff coverage is 86.5%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #376     +/-   ##
=======================================
+ Coverage   77.1%   77.3%   +0.1%     
=======================================
  Files         94      95      +1     
  Lines       4471    4483     +12     
=======================================
+ Hits        3449    3467     +18     
+ Misses       702     699      -3     
+ Partials     320     317      -3     
Impacted Files Coverage Δ
operator/builtin/parser/syslog/syslog.go 69.6% <ø> (ø)
operator/builtin/input/syslog/syslog.go 73.5% <83.3%> (+15.0%) ⬆️
operator/builtin/input/tcp/tcp.go 80.0% <100.0%> (+1.9%) ⬆️
operator/builtin/input/udp/udp.go 76.3% <100.0%> (+0.5%) ⬆️
operator/builtin/input/file/file.go 70.2% <0.0%> (-2.1%) ⬇️
operator/builtin/input/file/config.go 77.0% <0.0%> (ø)
operator/builtin/input/file/roller_other.go 100.0% <0.0%> (ø)
agent/builder.go 92.5% <0.0%> (+0.9%) ⬆️

The syslog_input operator was previously implemented
as a builder that built two operators. While this is
a perfectly fine solution, it happens to be the only
place in this codebase where multiple operators are
returned from a builder, excepting plugins.

This new implementation builds the same two operators,
but then wraps them together within a single operator.
This will allow for the eventual simplification of
other code, once plugins are also removed from this
codebase.
@djaglowski
Copy link
Member Author

The contrib failure is expected due to this change. I've proven out a change to syslogreceiver.DecodeInputConfig that will fix it:

// DecodeInputConfig unmarshals the input operator
func (f ReceiverType) DecodeInputConfig(cfg config.Receiver) (*operator.Config, error) {
	logConfig := cfg.(*SysLogConfig)
	yamlBytes, _ := yaml.Marshal(logConfig.Input)
	inputCfg := syslog.NewSyslogInputConfig("syslog_input")
	inputCfg.SyslogBaseConfig = syslogparser.NewSyslogParserConfig("syslog_parser").SyslogBaseConfig

	if err := yaml.Unmarshal(yamlBytes, &inputCfg); err != nil {
		return nil, err
	}
	return &operator.Config{Builder: inputCfg}, nil
}

@djaglowski
Copy link
Member Author

This change ended up simplifying the syslog_input operator in some notable ways. Particularly, I like that it removed the need for a custom unmarshal function.

@djaglowski djaglowski marked this pull request as ready for review February 11, 2022 20:00
@djaglowski djaglowski requested review from a team and jsirianni February 11, 2022 20:00
@jsirianni
Copy link
Member

@djaglowski do we expect contrib-tests to fail?

@djaglowski
Copy link
Member Author

@djaglowski do we expect contrib-tests to fail?

Yeah, see comment above.

Copy link
Member

@jsirianni jsirianni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I built this against my local contrib and tested TCP and UDP against rfc5424, looks good.

@djaglowski djaglowski merged commit 8e7555b into open-telemetry:main Feb 15, 2022
@djaglowski djaglowski deleted the syslog-input-as-wrapper branch February 15, 2022 18:01
@djaglowski djaglowski added this to the Reduce Complexity milestone Feb 18, 2022
jsirianni pushed a commit to jsirianni/opentelemetry-log-collection that referenced this pull request Mar 28, 2022
* Refactor syslog_input into a single operator

The syslog_input operator was previously implemented
as a builder that built two operators. While this is
a perfectly fine solution, it happens to be the only
place in this codebase where multiple operators are
returned from a builder, excepting plugins.

This new implementation builds the same two operators,
but then wraps them together within a single operator.
This will allow for the eventual simplification of
other code, once plugins are also removed from this
codebase.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants