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

More linter fixes and configurable logging #17

Merged
merged 9 commits into from
Mar 8, 2023
Merged

More linter fixes and configurable logging #17

merged 9 commits into from
Mar 8, 2023

Conversation

grcevski
Copy link
Contributor

@grcevski grcevski commented Mar 6, 2023

I added few things to be able to run the first benchmark on pingserver:

  • I re-enabled linting on pkg after fixing all of the issues the new linter version found.
  • I added a noop tracer to be used in the output pipe, so that we just consume the results and not print anything
  • I made the logging configurable for otelhttp and pingserver
  • I added a benchmark driver program for pingserver
  • I also made the build goal be the default for the Makefile. I hope you don't mind, I guess I'm used to make doing all the work and I kept forgetting to run build. It used to pick up prereqs as the first goal by default. So right now, just make does the full build.

Here's an example of how I run the otelhttp tracer without printing and WARN log level:

sudo PRINT_TRACES=false NOOP_TRACES=true EXECUTABLE_NAME=pingserver LOG_LEVEL=WARN ./otelhttp

@grcevski grcevski requested a review from mariomac March 6, 2023 19:29
@@ -57,7 +57,7 @@ func instrumentationPoints(elfF *elf.File, funcName string) (FuncOffsets, error)
if sec == nil {
return FuncOffsets{}, fmt.Errorf(".gosymtab section not found in target binary, make sure this is a Go application")
}
symTabRaw, err := sec.Data()
symTabRaw, _ := sec.Data()
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops! I forgot to handle the error. Instead of ignoring it, we should handle it:

if err != nil {
    return FuncOffsets{}, fmt.Errorf("getting memory section data: %w", err)
}

Copy link
Contributor

@mariomac mariomac left a comment

Choose a reason for hiding this comment

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

Thank you for doing this! I have a small suggestion

@grcevski grcevski requested a review from mariomac March 7, 2023 17:11
@grcevski
Copy link
Contributor Author

grcevski commented Mar 7, 2023

Done :)

@mariomac mariomac merged commit 6b1b211 into main Mar 8, 2023
@mariomac mariomac deleted the linter_fixes branch April 18, 2023 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants