Added Zap as the default Error log #11820

[tarun-kavipurapu]

Description

This Issue Fixes the Issue #11820
Fixes

• Made the Zap as the default ErrorLog Supplied to the Http server

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: tarun-kavipurapu / name: Tarun (57e4cf0, 5460f8d, a9f3aeb, e2b7c86, 71c16e0, 6582f98, f31a5f7, bc33333, f9c3375, 4caff5f)
• ✅ login: codeboten / name: Alex Boten (9a4d63d, 3a81191, 69577a7, 84c248c, c9ce37f)
• ✅ login: mx-psi / name: Pablo Baeyens (ebcc31e)

[bogdandrutu]

Suggested change

	// Setting the server error logger
	logger := zap.NewStdLog(settings.Logger)
	server := &http.Server{
	Handler: handler,
	ReadTimeout: hss.ReadTimeout,
	ReadHeaderTimeout: hss.ReadHeaderTimeout,
	WriteTimeout: hss.WriteTimeout,
	IdleTimeout: hss.IdleTimeout,
	ErrorLog: logger,
	server := &http.Server{
	Handler: handler,
	ReadTimeout: hss.ReadTimeout,
	ReadHeaderTimeout: hss.ReadHeaderTimeout,
	WriteTimeout: hss.WriteTimeout,
	IdleTimeout: hss.IdleTimeout,
	// Setting the server error logger
	ErrorLog: zap.NewStdLog(settings.Logger),

[bogdandrutu]

Please sign the CLA

[tarun-kavipurapu]

@bogdandrutu Made the desired Changes snd signed the CLA

[atoulme]

Please add a changelog.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.59%. Comparing base (cef6ce5) to head (ebcc31e).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #11830   +/-   ##
=======================================
  Coverage   91.59%   91.59%           
=======================================
  Files         449      449           
  Lines       23761    23763    +2     
=======================================
+ Hits        21763    21765    +2     
  Misses       1623     1623           
  Partials      375      375           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tarun-kavipurapu]

@atoulme change log added

[atoulme]

can we return the error?

[tarun-kavipurapu]

I dont think so because if errLog is nil in http.Server it will default to standard logger but if we return the error it may disrupt the entire flow In jaeger also similar pattern is followed
https://github.com/jaegertracing/jaeger/blob/8f612a876dc501e0db9786a054ff52fee314afb3/cmd/collector/app/server/http.go#L47

[yurishkuro]

it's fine to return the error, but the error handling won't be tested because the error will never happen - it only happens if the 2nd argument is not a valid log level, e.g. zapcore.Level(123)

[tarun-kavipurapu]

sure then will make a change returning the error

[atoulme]

it only matters if zap changes its internals moving forward, and since we're returning an error anyway, might as well return it.

[tarun-kavipurapu]

Hey Just a small question Unrelated to this
I want to contribute to the OpenTelemetry ecosystem actively. Which repositories would be most suitable for someone at a beginner level to start contributing effectively?

[atoulme]

that depends what you like to work on and what you need to get out of the project. Consider joining the CNCF slack and our SIG meetings to get involved.

[dmitryax]

@tarun-kavipurapu can you please try to resubmit the PR?

[tarun-kavipurapu]

@dmitryax is this what you mean by resubmitting PR or do you want me to create a new PR

[dmitryax]

@tarun-kavipurapu I meant creating another PR, but I can try merging this one again

[tarun-kavipurapu]

@dmitryax sure if this one fails i will create a new pr

[dmitryax]

@tarun-kavipurapu, it's still failing. Can you please try opening another PR?

[tarun-kavipurapu]

@dmitryax ya will do that and link the pr number here sorry i was a little busy today

[tarun-kavipurapu]

Made a new Pr here #11935