-
Notifications
You must be signed in to change notification settings - Fork 205
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
Bring up profiler in response to user enabling it #2683
Conversation
1a3235b
to
0fe8a72
Compare
0fe8a72
to
bbc5c9e
Compare
...c-monitor-core/src/main/java/com/microsoft/gcmonitor/notifications/NotificationObserver.java
Show resolved
Hide resolved
...c-monitor-core/src/main/java/com/microsoft/gcmonitor/notifications/NotificationObserver.java
Show resolved
Hide resolved
...i/src/main/java/com/microsoft/applicationinsights/alerting/config/AlertingConfiguration.java
Show resolved
Hide resolved
.../main/java/com/microsoft/applicationinsights/agent/internal/configuration/Configuration.java
Outdated
Show resolved
Hide resolved
.../com/microsoft/applicationinsights/agent/internal/profiler/PerformanceMonitoringService.java
Show resolved
Hide resolved
.../com/microsoft/applicationinsights/agent/internal/profiler/PerformanceMonitoringService.java
Show resolved
Hide resolved
...ain/java/com/microsoft/applicationinsights/agent/internal/profiler/ProfilingInitializer.java
Show resolved
Hide resolved
...ain/java/com/microsoft/applicationinsights/agent/internal/profiler/ProfilingInitializer.java
Outdated
Show resolved
Hide resolved
...ain/java/com/microsoft/applicationinsights/agent/internal/profiler/ProfilingInitializer.java
Show resolved
Hide resolved
3f6b78e
to
c758a0f
Compare
.../main/java/com/microsoft/applicationinsights/agent/internal/configuration/Configuration.java
Outdated
Show resolved
Hide resolved
...ng/src/main/java/com/microsoft/applicationinsights/agent/internal/init/SecondEntryPoint.java
Outdated
Show resolved
Hide resolved
.../com/microsoft/applicationinsights/agent/internal/profiler/PerformanceMonitoringService.java
Show resolved
Hide resolved
.../com/microsoft/applicationinsights/agent/internal/profiler/PerformanceMonitoringService.java
Show resolved
Hide resolved
.../com/microsoft/applicationinsights/agent/internal/profiler/config/ProfilerConfiguration.java
Show resolved
Hide resolved
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.
🎉
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.
LGTM
|
@@ -397,7 +407,7 @@ protected Set<Integer> getLivenessCheckPorts() { | |||
"-Dapplicationinsights.testing.global-ingestion-endpoint=" + FAKE_INGESTION_ENDPOINT); | |||
} | |||
if (REMOTE_DEBUG) { | |||
javaToolOptions.add("-agentlib:jdwp=transport=dt_socket,address=5005,server=y,suspend=y"); | |||
javaToolOptions.add("-agentlib:jdwp=transport=dt_socket,address=*:5005,server=y,suspend=y"); |
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.
was this needed on linux? ;-)
static class JavaProfilerConfiguredTest extends JavaProfileConfigTest { | ||
@RegisterExtension | ||
static final SmokeTestExtension testing = | ||
BASE_BUILDER.setProfilerEndpoint(ProfilerState.configuredEnabled).build(); |
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.
I'd suggest something like below, and construct the baseBuilder each time (since it's not immutable):
BASE_BUILDER.setProfilerEndpoint(ProfilerState.configuredEnabled).build(); | |
baseBuilder().setProfilerEndpoint(ProfilerState.configuredEnabled).build(); |
import javax.servlet.http.HttpServletRequest; | ||
import javax.servlet.http.HttpServletResponse; | ||
|
||
public class MockedProfilerSettingsServlet extends HttpServlet { |
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.
❤️❤️
Fix # .
For significant contributions please make sure you have completed the following items: