-
Notifications
You must be signed in to change notification settings - Fork 311
Experimental profiling support #1081
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1081 +/- ##
===========================================
- Coverage 78.04% 75.50% -2.54%
===========================================
Files 159 160 +1
Lines 15811 16386 +575
===========================================
+ Hits 12340 12373 +33
- Misses 3058 3587 +529
- Partials 413 426 +13 see 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
} | ||
hypot = math.Hypot(123.56789, 23.4567889) | ||
gamma3 = math.Gamma(3) | ||
xy = math.Pow(20, 3.5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put in the iteration value here to avoid possible optimization by the compiler.
newrelic.ProfilingTypeMutex| | ||
newrelic.ProfilingTypeThreadCreate| | ||
newrelic.ProfilingTypeBlock), | ||
newrelic.ConfigProfilingSampleInterval(time.Millisecond*500), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idea for docs: make recommendation for default value that comes out of performance testing
log.Printf("Profiling: %v every %v", c.Profiling.SelectedProfiles.Strings(), c.Profiling.Interval) | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a comment here explaining the need to increase the max samples stored limit
whatHappened, differences := jsondiff.Compare([]byte(expect), []byte(out), &o) | ||
t.Errorf("Config fields not as expected: %s:", whatHappened) | ||
t.Error(differences) | ||
//t.Error(expect) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's replace jsondiff with an internal tool
@@ -439,7 +439,7 @@ func TestAppProcess_ConnectChan_TraceObserverVariants(t *testing.T) { | |||
|
|||
select { | |||
case <-testApp.app.shutdownComplete: | |||
case <-time.After(2 * timeout): | |||
case <-time.After(80 * timeout): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was increased because the unit test was timing out and failing otherwise for some reason.
app.app.profiler.cpuSampleRateHz = hz | ||
app.app.profiler.lock.Unlock() | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look at whether there's a reasonable set of min/max limits to enforce for each of the configuration values and enforce what those are.
This PR brings the Go APM agent profiling code into the develop branch for additional testing and evaluation prior to being released.