Add all agent config to `agent.conf` file for convenient containerization use cases by kezhenxu94 · Pull Request #7470 · apache/skywalking

kezhenxu94 · 2021-08-17T13:10:55Z

If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #. NO
Update the CHANGES log.

codecov · 2021-08-17T13:29:26Z

Codecov Report

Merging #7470 (8e7cd5c) into master (39d23c1) will decrease coverage by 2.12%.
The diff coverage is n/a.

❗ Current head 8e7cd5c differs from pull request most recent head 05d6385. Consider uploading reports for the commit 05d6385 to get more accurate results

@@             Coverage Diff              @@
##             master    #7470      +/-   ##
============================================
- Coverage     58.81%   56.68%   -2.13%     
+ Complexity     4390     4363      -27     
============================================
  Files          1035     1075      +40     
  Lines         26592    27346     +754     
  Branches       2627     2711      +84     
============================================
- Hits          15640    15502     -138     
- Misses         9571    10482     +911     
+ Partials       1381     1362      -19

Impacted Files	Coverage Δ
...rver/storage/plugin/influxdb/query/AlarmQuery.java	`6.66% <0.00%> (-73.34%)`	⬇️
...ge/plugin/elasticsearch/query/AlarmQueryEsDAO.java	`5.71% <0.00%> (-71.43%)`	⬇️
...re/logging/core/converters/ThrowableConverter.java	`16.66% <0.00%> (-58.34%)`	⬇️
...apm/agent/core/remote/AuthenticationDecorator.java	`33.33% <0.00%> (-26.67%)`	⬇️
...r/storage/plugin/influxdb/query/EventQueryDAO.java	`42.06% <0.00%> (-16.67%)`	⬇️
...ge/plugin/elasticsearch/query/ESEventQueryDAO.java	`59.25% <0.00%> (-14.82%)`	⬇️
...m/commons/datacarrier/consumer/ConsumerThread.java	`80.55% <0.00%> (-11.12%)`	⬇️
...pm/agent/core/remote/EventReportServiceClient.java	`72.97% <0.00%> (-10.82%)`	⬇️
.../apm/agent/core/remote/LogReportServiceClient.java	`40.00% <0.00%> (-10.00%)`	⬇️
...p/server/core/analysis/metrics/PercentMetrics.java	`90.90% <0.00%> (-9.10%)`	⬇️
... and 53 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39d23c1...05d6385. Read the comment docs.

…tion use cases

wu-sheng

Once all tests passed, I am good with this.

mrproliu

I see some default value is not right with the document, I think we should update too.

mrproliu · 2021-08-17T15:29:02Z

+#  Timeout period of reading topics from the Kafka server, the unit is second. 
+plugin.kafka.get_topic_timeout=${SW_GET_TOPIC_TIMEOUT:10}
+#  Kafka producer configuration.
+plugin.kafka.consumer_config=${SW_PLUGIN_KAFKA_CONSUMER_CONFIG:}


Can't see where is using this config item?

I think it is in KafkaProducerManager#run.

@dmsolr Could you be clear about how to add this property? I mean for a hashmap type.

It is my mistake. The proper configuration item is plugin.kafka.producer_config.

The format is

plugin.kafka.producer_config[key]=value

and the key list was here.
I do not try to set it in JVM Options. It seems that you can't do it.

@kezhenxu94 Any preference to do this? This mode is hard to override through system env. We can't list them all, they are too many.

@kezhenxu94 Any preference to do this? This mode is hard to override through system env. We can't list them all, they are too many.

What about avoiding the key in the config key, for example plugin.kafka.producer_config=key1=value1,key2=value2, if we could use JSON in agent, we may use plugin.kafka.producer_config={"key":"val"}?

plugin.kafka.producer_config=key1=value1,key2=value2

I prefer to support this first in the agent initialization through another PR? @dmsolr Could you do this?

plugin.kafka.producer_config=key1=value1,key2=value2
It is good to me.

@dmsolr Could you do this?
Sure.

Please create an issue to track this.

wu-sheng · 2021-08-18T00:16:47Z

I rerun the Kafka e2e. The Kafka/Meter is always unstable :(

wu-sheng · 2021-08-18T00:57:31Z

@kezhenxu94 Could you check meter e2e? It seems failing again.

dashanji · 2021-09-19T13:02:36Z

I find some configurations are in configurations.md , but not in the agent.config , such as agent.trace_segment_ref_limit_per_span 、agent.instance_properties[key]=value . Otherwise , the configuration plugin.dubbo.consumer_provider_length_threshold in the agent.config is not the same as plugin.dubbo.provider_arguments_length_threshold in the configurations.md.

wu-sheng · 2021-09-19T13:06:37Z

I find some configurations are in configurations.md , but not in the agent.config , such as agent.trace_segment_ref_limit_per_span 、agent.instance_properties[key]=value . Otherwise , the configuration plugin.dubbo.consumer_provider_length_threshold in the agent.config is not the same as plugin.dubbo.provider_arguments_length_threshold in the configurations.md.

If there are names mismatch or missing, submit a pull request to fix directly.
Things like agent.instance_properties[key]=value are not pre-defined by SkyWalking, we don't know what to add.

kezhenxu94 · 2021-09-19T13:08:48Z

I find some configurations are in configurations.md , but not in the agent.config , such as agent.trace_segment_ref_limit_per_span 、agent.instance_properties[key]=value . Otherwise , the configuration plugin.dubbo.consumer_provider_length_threshold in the agent.config is not the same as plugin.dubbo.provider_arguments_length_threshold in the configurations.md.

@dashanji can you open a pull request to fix this? For agent.instance_properties[xxx], I think we also need to migrate to the format of agent.instance_properties_json={}. plugin.dubbo.consumer_provider_length_threshold should be typo of plugin.dubbo.consumer_length_threshold

dashanji · 2021-09-19T13:14:15Z

I find some configurations are in configurations.md , but not in the agent.config , such as agent.trace_segment_ref_limit_per_span 、agent.instance_properties[key]=value . Otherwise , the configuration plugin.dubbo.consumer_provider_length_threshold in the agent.config is not the same as plugin.dubbo.provider_arguments_length_threshold in the configurations.md.

@dashanji can you open a pull request to fix this? For agent.instance_properties[xxx], I think we also need to migrate to the format of agent.instance_properties_json={}. plugin.dubbo.consumer_provider_length_threshold should be typo of plugin.dubbo.consumer_length_threshold

OK.

dashanji · 2021-09-19T13:15:15Z

I find some configurations are in configurations.md , but not in the agent.config , such as agent.trace_segment_ref_limit_per_span 、agent.instance_properties[key]=value . Otherwise , the configuration plugin.dubbo.consumer_provider_length_threshold in the agent.config is not the same as plugin.dubbo.provider_arguments_length_threshold in the configurations.md.

If there are names mismatch or missing, submit a pull request to fix directly.
Things like agent.instance_properties[key]=value are not pre-defined by SkyWalking, we don't know what to add.

OK , I will open a pull request to fix it.

wu-sheng · 2021-09-19T13:32:27Z

agent.instance_properties_json={} should be a new feature, rather than a fix. @dashanji If you like, please submit another new(not the wrong names fix) PR to add this.

dashanji · 2021-09-19T14:33:35Z

agent.instance_properties_json={} should be a new feature, rather than a fix. @dashanji If you like, please submit another new(not the wrong names fix) PR to add this.

Sorry I don't know much about java, so I can't realize this feature at present.

wu-sheng · 2021-09-19T14:43:29Z

agent.instance_properties_json={} should be a new feature, rather than a fix. @dashanji If you like, please submit another new(not the wrong names fix) PR to add this.

Sorry I don't know much about java, so I can't realize this feature at present.

OK, then if we need someone to add this feature first, otherwise, agent.instance_properties[key]=value can't be added, just purely because we don't know what to add.

But others you proposed, feel free to send a pull request to fix.

kezhenxu94 added agent Language agent related. chore Chores about the project, like code cleaning up, typos, upgrading dependencies, etc. labels Aug 17, 2021

kezhenxu94 added this to the 8.8.0 milestone Aug 17, 2021

kezhenxu94 requested review from mrproliu and wu-sheng August 17, 2021 13:10

wu-sheng reviewed Aug 17, 2021

View reviewed changes

Comment thread CHANGES.md

kezhenxu94 force-pushed the chore/agent-config branch 2 times, most recently from bc6de0f to 0abea49 Compare August 17, 2021 13:39

Add all agent config to agent.conf file for convenient containeriza…

6d4de99

…tion use cases

kezhenxu94 force-pushed the chore/agent-config branch from 0abea49 to 6d4de99 Compare August 17, 2021 14:13

wu-sheng previously approved these changes Aug 17, 2021

View reviewed changes

mrproliu reviewed Aug 17, 2021

View reviewed changes

Comment thread apm-sniffer/config/agent.config Outdated

Comment thread apm-sniffer/config/agent.config Outdated

Comment thread apm-sniffer/config/agent.config Outdated

kezhenxu94 dismissed wu-sheng’s stale review via 90acd98 August 17, 2021 15:15

kezhenxu94 force-pushed the chore/agent-config branch from 90acd98 to 038199b Compare August 17, 2021 15:21

mrproliu reviewed Aug 17, 2021

View reviewed changes

kezhenxu94 force-pushed the chore/agent-config branch from 038199b to 738f6b8 Compare August 17, 2021 15:39

wu-sheng requested a review from mrproliu August 18, 2021 00:05

wu-sheng previously approved these changes Aug 18, 2021

View reviewed changes

Fix name and default value

05d6385

kezhenxu94 dismissed wu-sheng’s stale review via 05d6385 August 18, 2021 01:10

kezhenxu94 force-pushed the chore/agent-config branch from 738f6b8 to 05d6385 Compare August 18, 2021 01:10

wu-sheng approved these changes Aug 18, 2021

View reviewed changes

mrproliu approved these changes Aug 18, 2021

View reviewed changes

wu-sheng merged commit 6bb4d35 into master Aug 18, 2021

wu-sheng deleted the chore/agent-config branch August 18, 2021 02:25

wu-sheng modified the milestones: 8.8.0, Java - 8.8.0 Aug 27, 2021

kezhenxu94 mentioned this pull request Sep 22, 2021

[Feature] set instance properties in json format: agent.instance_properties_json #7762

Closed

3 tasks

Conversation

kezhenxu94 commented Aug 17, 2021

Uh oh!

codecov Bot commented Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

wu-sheng left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrproliu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wu-sheng commented Aug 18, 2021

Uh oh!

wu-sheng commented Aug 18, 2021

Uh oh!

dashanji commented Sep 19, 2021

Uh oh!

wu-sheng commented Sep 19, 2021

Uh oh!

kezhenxu94 commented Sep 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dashanji commented Sep 19, 2021

Uh oh!

dashanji commented Sep 19, 2021

Uh oh!

wu-sheng commented Sep 19, 2021

Uh oh!

dashanji commented Sep 19, 2021

Uh oh!

wu-sheng commented Sep 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Aug 17, 2021 •

edited

Loading

kezhenxu94 commented Sep 19, 2021 •

edited

Loading