Add agent self-observability. #716

weixiang1862 · 2024-09-13T08:20:58Z

Add agent self-observability.

If this is non-trivial feature, paste the links/URLs to the design doc.
Update the documentation to include this new feature.
Tests(including UT, IT, E2E) are added to verify the new feature.
If it's UI related, attach the screenshots below.
If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<12595> agent part.
Update the CHANGES log.

wu-sheng · 2024-09-13T09:38:22Z

You need to fix CIs.

wu-sheng · 2024-09-13T09:53:58Z

And we need to update docs about agent self dashboard.

weixiang1862 · 2024-09-13T10:18:02Z

docker compose scenarios all failed with scenario.sh: line 35: docker-compose: command not found, how can i fix this?

wu-sheng · 2024-09-13T10:44:24Z

I think docker compose version changed?

wu-sheng · 2024-09-13T13:49:39Z

.github/actions/run/action.yml

+    - name: Install docker-compose
+      shell: bash
+      if: runner.os != 'Windows'
+      run: |
+        if ! command docker-compose 2>&1 > /dev/null; then
+          echo "Installing docker-compose"
+          sudo curl -L "https://github.com/docker/compose/releases/download/1.29.2/docker-compose-$(uname -s)-$(uname -m)" -o /usr/local/bin/docker-compose
+          sudo chmod +x /usr/local/bin/docker-compose
+        fi


@kezhenxu94 Is this a good practice?

This is not a good choice, we did this because our infra e2e uses the docker-compose in third party SDK (testcontainers), that we cannot easily fix. Please fix as #716 (comment)

OK, I will revert this change.

wu-sheng · 2024-09-13T15:18:52Z

test/plugin/scenarios/agent-so11y-scenario/config/expectedData.yaml

+      name: created_ignored_context_counter
+      tags:
+      - {name: created_by, value: propagated}
+    singleValue: 1.0


Is counter value 1 stable?

Yes, this propagated context is created by new URL("http://localhost:8080/agent-so11y-scenario/case/propagated").getContent();

wu-sheng · 2024-09-13T15:20:48Z

...ffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/so11y/AgentSO11Y.java

+    private static final Counter PROPAGATED_CONTEXT_COUNTER = MeterFactory
+        .counter("created_tracing_context_counter").tag("created_by", "propagated").build();


We should use the get method and lazy initialization. Because initializing through static field/block, could make some classes loaded before expected.

wu-sheng · 2024-09-13T15:22:32Z

...src/main/java/org/apache/skywalking/apm/agent/core/so11y/bootstrap/BootstrapPluginSO11Y.java

+package org.apache.skywalking.apm.agent.core.so11y.bootstrap;
+
+public interface BootstrapPluginSO11Y {
+    void recordInterceptorTimeCost(double timeCostInNanos);


Suggested change

void recordInterceptorTimeCost(double timeCostInNanos);

void duration(double timeCostInNanos);

wu-sheng · 2024-09-13T15:22:45Z

...src/main/java/org/apache/skywalking/apm/agent/core/so11y/bootstrap/BootstrapPluginSO11Y.java

+public interface BootstrapPluginSO11Y {
+    void recordInterceptorTimeCost(double timeCostInNanos);
+
+    void recordInterceptorError(String pluginName, String interType);


Suggested change

void recordInterceptorError(String pluginName, String interType);

void error(String pluginName, String interType);

wu-sheng · 2024-09-13T15:27:20Z

...src/main/java/org/apache/skywalking/apm/agent/core/so11y/bootstrap/BootstrapPluginSO11Y.java

+
+package org.apache.skywalking.apm.agent.core.so11y.bootstrap;
+
+public interface BootstrapPluginSO11Y {


Suggested change

public interface BootstrapPluginSO11Y {

public interface BootstrapPluginSo11y {

wu-sheng · 2024-09-13T15:27:32Z

...in/java/org/apache/skywalking/apm/agent/core/so11y/bootstrap/BootstrapPluginSO11YBridge.java

+ * used by {@link BootstrapInterRuntimeAssist}
+ */
+@SuppressWarnings("unused")
+public class BootstrapPluginSO11YBridge implements BootstrapPluginSO11Y {


Suggested change

public class BootstrapPluginSO11YBridge implements BootstrapPluginSO11Y {

public class BootstrapPluginSo11yBridge implements BootstrapPluginSO11Y {

wu-sheng · 2024-09-13T15:32:12Z

...ffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/so11y/AgentSO11Y.java

+        .steps(Arrays.asList(0.01d, 0.1d, 0.5d, 1d, 3d, 5d, 10d, 50d, 100d, 200d, 500d, 1000d))
+        .build();
+
+    // interceptor error counter


Suggested change

// interceptor error counter

// A map to cache meter obj(s) for plugins. The key is the plugin name.

wu-sheng · 2024-09-13T15:34:36Z

...ffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/so11y/AgentSO11Y.java

+    // interceptor error counter
+    private static final Map<String, Counter> COUNTER_CACHE = new ConcurrentHashMap<>();
+
+    public static void recordTracingContextCreate(boolean forceSampling, boolean ignoredTracingContext) {


Suggested change

public static void recordTracingContextCreate(boolean forceSampling, boolean ignoredTracingContext) {

public static void measureTracingContextCreation(boolean forceSampling, boolean ignoredTracingContext) {

wu-sheng · 2024-09-13T15:35:14Z

...ffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/so11y/AgentSO11Y.java

+        }
+    }
+
+    public static void recordTracingContextFinish(boolean ignoredTracingContext) {


Suggested change

public static void recordTracingContextFinish(boolean ignoredTracingContext) {

public static void measureTracingContextCompletion(boolean ignoredTracingContext) {

wu-sheng · 2024-09-13T15:35:36Z

...ffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/so11y/AgentSO11Y.java

+        }
+    }
+
+    public static void recordLeakedTracingContext(boolean ignoredTracingContext) {


Suggested change

public static void recordLeakedTracingContext(boolean ignoredTracingContext) {

public static void measureLeakedTracingContext(boolean ignoredTracingContext) {

wu-sheng · 2024-09-13T15:35:56Z

...ffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/so11y/AgentSO11Y.java

+        }
+    }
+
+    public static void recordInterceptorTimeCost(double timeCostInNanos) {


Suggested change

public static void recordInterceptorTimeCost(double timeCostInNanos) {

public static void durationOfInterceptor(double timeCostInNanos) {

wu-sheng · 2024-09-13T15:36:17Z

...ffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/so11y/AgentSO11Y.java

+        INTERCEPTOR_TIME_COST.addValue(timeCostInNanos / 1000000);
+    }
+
+    public static void recordInterceptorError(String pluginName, String interType) {


Suggested change

public static void recordInterceptorError(String pluginName, String interType) {

public static void errorOfPlugin(String pluginName, String interType) {

wu-sheng · 2024-09-13T15:38:24Z

...a/org/apache/skywalking/apm/agent/core/plugin/interceptor/enhance/v2/InstMethodsInterV2.java

    private InstanceMethodsAroundInterceptorV2 interceptor;

-    public InstMethodsInterV2(String instanceMethodsAroundInterceptorClassName, ClassLoader classLoader) {
+    private static final String INTERCEPTOR_TYPE = "inst";


Please move all static final field at the top.

wu-sheng · 2024-09-13T15:53:22Z

...e/skywalking/apm/agent/core/plugin/interceptor/enhance/InstMethodsInterWithOverrideArgs.java

@@ -70,12 +75,16 @@ public Object intercept(@This Object obj, @AllArguments Object[] allArguments, @
        @Morph OverrideCallable zuper) throws Throwable {
        EnhancedInstance targetObject = (EnhancedInstance) obj;

+        long interceptorTimeCost = 0L;
+        long beforeStartTime = System.nanoTime();


Suggested change

long beforeStartTime = System.nanoTime();

long startTimeOfMethodBeforeInter = System.nanoTime();

Let's use this naming style. beforeStartTime like before start time, which is not our meaning.

kezhenxu94 · 2024-09-13T15:58:37Z

docker compose scenarios all failed with scenario.sh: line 35: docker-compose: command not found, how can i fix this?

Please change all docker-compose command in scenario.sh to docker compose

wu-sheng · 2024-09-14T03:47:11Z

https://skywalking.apache.org/docs/main/next/en/setup/backend/dashboards-so11y-java-agent/ → Status: 404

I think we don't have a pull request for that?

weixiang1862 · 2024-09-14T03:50:03Z

https://skywalking.apache.org/docs/main/next/en/setup/backend/dashboards-so11y-java-agent/ → Status: 404

I think we don't have a pull request for that?

Yes, it will be submit this weekend.

wu-sheng · 2024-09-14T03:52:47Z

docs/menu.yml

-      - name: "Plugin development guide"
+      - name: "Agent Self Observability"
+        path: "/en/setup/service-agent/java-agent/Java-Agent-self-observability"
+      - name: "Plugin Development Guide"


Could you add Plugin Test doc next to this? I think we missed it from menu.

wu-sheng · 2024-09-14T03:53:00Z

docs/en/setup/service-agent/java-agent/Java-Agent-self-observability.md

@@ -0,0 +1,13 @@
+# Java Agent self-observability


Suggested change

# Java Agent self-observability

# Agent Self Observability

docs/en/setup/service-agent/java-agent/Java-Agent-self-observability.md

wu-sheng · 2024-09-14T06:50:23Z

...ffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/so11y/AgentSo11y.java

+        if (INTERCEPTOR_TIME_COST == null) {
+            INTERCEPTOR_TIME_COST = MeterFactory
+                .histogram("tracing_context_performance")
+                .steps(Arrays.asList(0.01d, 0.1d, 0.5d, 1d, 3d, 5d, 10d, 50d, 100d, 200d, 500d, 1000d))


Arrays.asList should be executed once. Please make them a static variable.

wu-sheng · 2024-09-14T06:51:01Z

...ffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/so11y/AgentSo11y.java

+                .steps(Arrays.asList(0.01d, 0.1d, 0.5d, 1d, 3d, 5d, 10d, 50d, 100d, 200d, 500d, 1000d))
+                .build();
+        }
+        INTERCEPTOR_TIME_COST.addValue(timeCostInNanos / 1000000);


Metrics should be raw data, right? I don't think a method of interceptor would cost over 1 ms?

Do you have concern of long overflow?

How does the value look like in your testing?

It is a group of time cost(nanos) in agent-so11y-scenario, we should update histogram steps too, right?

4453.0 1.5740867E7 1875.0 3276344.0 1923509.0 2316475.0 932202.0 37576.0 1706292.0 1846.0 303985.0 126229.0 3075.0 203682.0 1919.0 29810.0 2636649.0

Yes, I think they should be aligned. Consider 4k ns, I think to keep ns as unit makes sense. Agree?

Yes, ns as unit make sense.

Let's keep the first buckets of histogram as 1000ns.

Let's keep the first buckets of histogram as 1000ns.

Is this step ok?

private static final List<Double> TIME_COST_HISTOGRAM_STEPS = Arrays.asList( 1000d, 10000d, 50000d, 100000d, 300000d, 500000d, 1000000d, 5000000d, 10000000d, 20000000d, 50000000d, 100000000d );

The max is 100ms per intercepting. I think it is a very bad performance.

Let's do this anyway. I am not sure how bad an interceptor would be.

wu-sheng · 2024-09-15T08:48:13Z

docs/en/setup/service-agent/java-agent/Agent-self-observability.md

+- `created_ignored_context_counter` and `finished_ignored_context_counter`. Same concepts like `*_tracing_context_counter`.
+- `interceptor_error_counter` - Counter. The number of errors happened in the interceptor logic, with `label=plugin_name, inter_type(constructor, inst, static)`. We don't add interceptor names into labels in case of OOM. The number of plugins is only dozens, it is predictable, but the number of interceptors will be hundreds.
+- `possible_leaked_context_counter` - Counter. The number of detected leaked contexts. It should include the `label=source(value=tracing, ignore)`. When `source=tracing`, it is today's shadow tracing context. But now, it is measured.
+- `tracing_context_performance` - Histogram. For successfully finished tracing context, it measures every interceptor's time cost(by using nanoseconds), the buckets of the histogram are {0.001, 0.01, 0.05, 0.1, 0.3, 0.5, 1, 5, 10, 20, 50, 100}ms. This provides the performance behavior for the tracing operations.


Here, we should use ns as unit, right?

wu-sheng · 2024-09-15T09:25:45Z

CI fails, please fix.

Add agent self-observability.

9bc3a7b

wu-sheng added enhancement New feature or request core feature labels Sep 13, 2024

wu-sheng added this to the 9.4.0 milestone Sep 13, 2024

Bump up GitHub artifact action version.

5745d42

Add docker-compose install step in github workflow.

68e166f

wu-sheng reviewed Sep 13, 2024

View reviewed changes

weixiang added 2 commits September 14, 2024 10:29

Changes follow suggestion.

dfd0f1e

Add Java Agent self-observability docs.

5ebe4f3

Update menu.yml

2f249e2

wu-sheng reviewed Sep 14, 2024

View reviewed changes

docs/en/setup/service-agent/java-agent/Java-Agent-self-observability.md Outdated Show resolved Hide resolved

weixiang added 2 commits September 14, 2024 12:07

Update docs.

3a18ae7

Update docs.

9bac4bb

wu-sheng reviewed Sep 14, 2024

View reviewed changes

Use ns as tracing_context_performance unit.

14fee93

wu-sheng reviewed Sep 15, 2024

View reviewed changes

Update docs.

1464daa

wu-sheng mentioned this pull request Sep 15, 2024

Add SkyWalking Java Agent self observability dashboard apache/skywalking#12622

Merged

4 tasks

wu-sheng approved these changes Sep 15, 2024

View reviewed changes

wu-sheng merged commit 7536424 into apache:main Sep 15, 2024
191 of 192 checks passed

		private static final Counter PROPAGATED_CONTEXT_COUNTER = MeterFactory
		.counter("created_tracing_context_counter").tag("created_by", "propagated").build();

	void recordInterceptorTimeCost(double timeCostInNanos);
	void duration(double timeCostInNanos);

	void recordInterceptorError(String pluginName, String interType);
	void error(String pluginName, String interType);


		package org.apache.skywalking.apm.agent.core.so11y.bootstrap;

		public interface BootstrapPluginSO11Y {

	public class BootstrapPluginSO11YBridge implements BootstrapPluginSO11Y {
	public class BootstrapPluginSo11yBridge implements BootstrapPluginSO11Y {

	// interceptor error counter
	// A map to cache meter obj(s) for plugins. The key is the plugin name.

	public static void recordTracingContextCreate(boolean forceSampling, boolean ignoredTracingContext) {
	public static void measureTracingContextCreation(boolean forceSampling, boolean ignoredTracingContext) {

	public static void recordTracingContextFinish(boolean ignoredTracingContext) {
	public static void measureTracingContextCompletion(boolean ignoredTracingContext) {

	public static void recordLeakedTracingContext(boolean ignoredTracingContext) {
	public static void measureLeakedTracingContext(boolean ignoredTracingContext) {

	public static void recordInterceptorTimeCost(double timeCostInNanos) {
	public static void durationOfInterceptor(double timeCostInNanos) {

	public static void recordInterceptorError(String pluginName, String interType) {
	public static void errorOfPlugin(String pluginName, String interType) {

	long beforeStartTime = System.nanoTime();
	long startTimeOfMethodBeforeInter = System.nanoTime();

Add agent self-observability. #716

Add agent self-observability. #716

Conversation

weixiang1862 commented Sep 13, 2024

Add agent self-observability.

wu-sheng commented Sep 13, 2024

wu-sheng commented Sep 13, 2024

weixiang1862 commented Sep 13, 2024

wu-sheng commented Sep 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wu-sheng Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kezhenxu94 commented Sep 13, 2024

wu-sheng commented Sep 14, 2024

weixiang1862 commented Sep 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wu-sheng commented Sep 15, 2024

wu-sheng Sep 13, 2024 •

edited

Loading