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

go.mod: upgrade datadog-agent/pkg/obfuscate #1774

Merged
merged 2 commits into from
Mar 17, 2023
Merged

Conversation

felixge
Copy link
Member

@felixge felixge commented Mar 3, 2023

Fixes #1153

What does this PR do?

go get github.com/DataDog/datadog-agent/pkg/obfuscate@v0.43.0
go get github.com/outcaste-io/ristretto@v0.2.1
go mod tidy

Motivation

Describe how to test/QA your changes

Reviewer's Checklist

  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • Changed code has unit tests for its functionality.
  • If this interacts with the agent in a new way, a system test has been added.

@felixge felixge added this to the v1.49.0 milestone Mar 3, 2023
@pr-commenter
Copy link

pr-commenter bot commented Mar 3, 2023

Benchmarks

Comparing candidate commit 52f768f in PR branch felix.geisendoerfer/gh-1153 with baseline commit 10e9cce in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 18 metrics, 0 unstable metrics.

@nsrip-dd
Copy link
Contributor

nsrip-dd commented Mar 3, 2023

I know this is marked as a draft, but FWIW I don't think this gets rid of glog. The following program, built against this branch, still gets the glog flags:

package main

import (
	"flag"
	_ "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
)

func main() {
	flag.PrintDefaults()
}

Output:

$ go run main.go
  -alsologtostderr
    	log to standard error as well as files
  -log_backtrace_at value
    	when logging hits line file:N, emit a stack trace
  -log_dir string
    	If non-empty, write log files in this directory
  -logtostderr
    	log to standard error instead of files
  -stderrthreshold value
    	logs at or above this threshold go to stderr
  -v value
    	log level for V logs
  -vmodule value
    	comma-separated list of pattern=N settings for file-filtered logging

Looks like the obfuscate v0.43.0 module got the wrong version of the ristretto dependency (see this issue). But we can upgrade like so to get the right one:

go get github.com/outcaste-io/ristretto@v0.2.1

If I do that against this branch, the glog flags go away.

@felixge felixge force-pushed the felix.geisendoerfer/gh-1153 branch from 040d275 to d188fa2 Compare March 11, 2023 15:32
@felixge felixge marked this pull request as ready for review March 11, 2023 15:45
@felixge felixge requested a review from a team as a code owner March 11, 2023 15:45
@felixge
Copy link
Member Author

felixge commented Mar 11, 2023

I know this is marked as a draft, but FWIW I don't think this gets rid of glog.

Yeah, I hadn't even checked for that yet 😅 . I got distracted by the performance regression that was reported.

But we can upgrade like so to get the right one:

go get github.com/outcaste-io/ristretto@v0.2.1

If I do that against this branch, the glog flags go away.

Thanks. I applied this and updated the patch. It seems to work and also makes the perf regression go away and is now reporting an improvement 🤷‍♂️😄 .

This is ready for review.

@felixge
Copy link
Member Author

felixge commented Mar 15, 2023

Thanks for review Nick.

I'm going to wait for #1798 before landing this because I want to see the profile for the performance improvement reported on this PR.

@felixge felixge merged commit e67e56e into main Mar 17, 2023
@felixge felixge deleted the felix.geisendoerfer/gh-1153 branch March 17, 2023 15:38
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.

v1.35.0 breaks any program trying to use a -v flag
2 participants