Report the agent version to OAP

[nisiyong]

Report the agent version to OAP

Currently, the agent version used by all applications cannot be sensed, It is beneficial for agent developers to diagnose some problems, and help us find which applications haven't upgraded the agent. I use git-commit-id-maven-plugin to generate the agent version, and report it to OAP when the agent starts.

The agent version is consist of three parts:

1. build version: the maven project version
2. short commit id: git commit short id
3. date: build date.

Here is the agent log from my local env with this feature:

INFO 2022-11-24 09:51:32.331 main InstanceJsonPropertiesUtil : SkyWalking agent version: 8.14.0-SNAPSHOT-add2bc4-221124

And the OAP server could show this agent version, too. What do you think of this feature? Any suggestions? @wu-sheng

• 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 #.
• Update the CHANGES log.

[wu-sheng]

date: build date.

Date of build failed in our test in the OAP side. So, we only have version + commit.

Ref the main repo version number generator.

cc @kezhenxu94

[kezhenxu94]

I remember we have already reported the agent version to OAP long time ago, although we didn't persist the version into storage.

[nisiyong]

I remember we have already reported the agent version to OAP long time ago, although we didn't persist the version into storage.

Where? I think this is helpful, agent version should persist with the instance info.

[kezhenxu94]

This PR also doesn't take the source release tar ball into consideration, if the users download our source tar (without git metadata at all), and build from source, the version number is undefined in this case, please also refer to https://github.com/apache/skywalking/pull/8004/files#diff-86b1ad012d67ec23f3a775179150ce4d2af54836a4156c0921aad37f2c671a05

[nisiyong]

This PR also doesn't take the source release tar ball into consideration

It's easy to solve, we could use the maven-ant-plugin to copy this file to the skywalking agent distribution folder.

[kezhenxu94]

I remember we have already reported the agent version to OAP long time ago, although we didn't persist the version into storage.

Where? I think this is helpful, agent version should persist with the instance info.

Read the following class

skywalking-java/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/remote/AgentIDDecorator.java

Lines 41 to 42 in 70b3e88

	public class AgentIDDecorator implements ChannelDecorator {
	private static final ILog LOGGER = LogManager.getLogger(AgentIDDecorator.class);

[nisiyong]

@kezhenxu94 OKay, thanks. I take a look first.

[nisiyong]

@wu-sheng @kezhenxu94
After reading AgentIDDecorator.java, I still think this PR implementation will be better. There are some reasons:

1. Who will care about the agent version? Obviously, the skywalking maintainer in each company. If they don't need to develop their own plugins, they could download the agent distribution which has the skywalking-agent-git.properties, why do they need to build from the source code tarball, rather than from the Github repo? If they do that, just let the version set Unkown, I think it is fine for them.
2. The AgentIDDecorator doesn't contain the git commit id which is important to help the maintainers identify all agent versions built from the related source code. Also, we don't need to report the agent version in each client request, I think what AgentIDDecorator implementation is more like the network protocol versions in each request.
3. Bind the version with instance heartbeat, it will be easy to maintain, if we upgrade the agent, after the application restarts, we could identify it has been upgraded quickly.

Thank you all, if you have any suggestions, please let me know.

[kezhenxu94]

1. Who will care about the agent version? Obviously, the skywalking maintainer in each company. If they don't need to develop their own plugins, they could download the agent distribution which has the skywalking-agent-git.properties, why do they need to build from the source code tarball, rather than from the Github repo? If they do that, just let the version set Unkown, I think it is fine for them.

There is no reason for us to doubt why they build from source code tarball, we as an Apache project are mandatory to provide source tarball and guarantee that users can use the project after compilation from that tarball. I'm totally fine to let the version be Unknown but please document this because this is stored in storage and (maybe) UI would display this someday, leaving the version to be Unknown should bring confusion to users and unnecessary bug report to the community unless clearly documented.

2. The AgentIDDecorator doesn't contain the git commit id which is important to help the maintainers identify all agent versions built from the related source code. Also, we don't need to report the agent version in each client request, I think what AgentIDDecorator implementation is more like the network protocol versions in each request.

AgentIDDecorator was indeed introduced as a network protocol version for, i.e. cloud vendors, looks like we cannot reuse this part, I'm ok to accept this PR once other comments are addressed.

[wu-sheng]

be Unknown but please document this because this is stored in storage and (maybe) UI would display this someday, l

As an instance property, it is on the UI already. Instance table includes the attribute column to list this.

[nisiyong]

please document this because this is stored in storage and (maybe) UI would display this someday, leaving the version to be Unknown should bring confusion to users and unnecessary bug report to the community unless clearly documented.

@kezhenxu94
Thanks your comment, I think there is another way. If they build from the source tarball, we could try to report only the build version without git properties file, maybe we could do it like AgentIDDecorator did.

[wu-sheng]

If we need this version property, then source tar should be covered.
We should make sure the version file would be generated when run source release command.
Unknown is not correct for an official release.
.git file should be a way to generate version, but source release tar ball is static, we should have a solution for that.

[nisiyong]

Okay, I will find the solution for source tarball recently.

[kezhenxu94]

Okay, I will find the solution for source tarball recently.

If you have no better idea, take a look at how OAP did this

[nisiyong]

@kezhenxu94 Thanks, I will take a look.

[nisiyong]

This PR I almost finish. Two more things I need to confirm:

1. Are you agree with the agent version format, like 8.14.0-2ef1977-221126?
2. Now we could copy the version properties into source tarball, but it will compile failed because of the check style plugin, I think this file needs to be ignored.

[INFO] --- git-commit-id-plugin:4.9.10:revision (get-the-git-infos) @ apm-agent-core ---
[INFO] 
[INFO] --- maven-checkstyle-plugin:3.1.0:check (validate) @ apm-agent-core ---
[INFO] Starting audit...
[ERROR] /private/tmp/apache-skywalking-java-agent-8.14.0-test/apm-sniffer/apm-agent-core/src/main/resources/skywalking-agent-version.properties:1: Missing a header - not enough lines in file. [RegexpHeader]
Audit done.
[INFO] There is 1 error reported by Checkstyle 8.19 with /tmp/apache-skywalking-java-agent-8.14.0-test/apm-checkstyle/checkStyle.xml ruleset.
[ERROR] src/main/resources/skywalking-agent-version.properties:[1] (header) RegexpHeader: Missing a header - not enough lines in file.
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for java-agent 8.14.0-SNAPSHOT:
[INFO] 
[INFO] java-agent ......................................... SUCCESS [  2.242 s]
[INFO] java-agent-commons ................................. SUCCESS [  0.126 s]
[INFO] apm-util ........................................... SUCCESS [  2.625 s]
[INFO] java-agent-datacarrier ............................. SUCCESS [  2.314 s]
[INFO] java-agent-protocol ................................ SUCCESS [  0.057 s]
[INFO] java-agent-network ................................. SUCCESS [  6.497 s]
[INFO] java-agent-sniffer ................................. SUCCESS [  0.048 s]
[INFO] apm-agent-core ..................................... FAILURE [  3.636 s]
[INFO] apm-agent .......................................... SKIPPED
[INFO] apm-test-tools ..................................... SKIPPED
[INFO] apm-sdk-plugin ..................................... SKIPPED
[INFO] rabbitmq-plugin .................................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  18.280 s
[INFO] Finished at: 2022-11-26T23:52:41+08:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (validate) on project apm-agent-core: You have 1 Checkstyle violation. -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :apm-agent-core

[wu-sheng]

1. This should be fine. I don't have any objection.
2. We need to confirm whether OAP faced issue has been resolved. The OAP used a similar way, but the last part(date) is not updated properly.

[nisiyong]

2. The OAP used a similar way, but the last part(date) is not updated properly.

@wu-sheng @kezhenxu94 Once the version properties file has been generated, the git.build.time indeed couldn't be overwritten unless this file was clean. I think the reason relates to this PR git-commit-id/git-commit-id-maven-plugin#513.

When we package the agent with .git, we run will run maven clean phrase first, this file will generate every time, so the git.build.time is the newest. I think it is okay, we can still keep it.

When we package the agent from the source tarball, as the git.build.time couldn't be written, I think we could ignore this field if we haven't better solutions. We could remove the git.build.time line in the create_release.sh after the initialize, and the Version#toString could identify whether the build time is blank.

[wu-sheng]

I think we should have two files, one for commit id, the other is for final version number. Then we could make sure the version suffix is correct.

[nisiyong]

I think we should have two files, one for commit id, the other is for final version number. Then we could make sure the version suffix is correct.

Could you explain more about why we need two files? This PR is finished, please consider this implementation. Now we could generate the agent version in the git repo scenario and the source tarball scenario. And the build time will be ignored when users build the agent from the source tarball.

Here are the agent logs I tested.

1. Agent log, built from git repo

INFO 2022-11-27 22:54:53.642 main Version : SkyWalking agent version: 8.14.0-SNAPSHOT-4166be4-20221127145403

if we run mvn clean package to package the agent again, the build time is changed, the new agent log as follows:

INFO 2022-11-27 22:57:28.241 main Version : SkyWalking agent version: 8.14.0-SNAPSHOT-4166be4-20221127145649

2. Agent log, built from source tarball
Use the create_release.sh to package the source tarball.

INFO 2022-11-27 22:48:10.600 main Version : SkyWalking agent version: 8.14.0-SNAPSHOT-4166be4

[wu-sheng]

I want to keep both scenarios having the same version style. Because in the ASF, only the source tar build is the real release.

[nisiyong]

I want to keep both scenarios having the same version style. Because in the ASF, only the source tar build is the real release.

Okay, If you insist keep them have the same style, I think buildVersion + commitId is enough for me, the buildTime just helps me identify the version easier.

[wu-sheng]

When we package the source tar, we should ship the commit into a file. Then when building binary from source tar, it should generate the same version.
Of course, if the user changes the source codes on their own, that is not our concern.

[wu-sheng]

I think buildVersion + commitId is enough for me

This works for me too.

[nisiyong]

@wu-sheng @kezhenxu94 The git.build.time was removed, thank you all, please take a look.

BTW, what the create_release.sh does is very smart 👍, generate the version.properties first then copy it to the source tarball, I haven't thought about it ever.

[wu-sheng]

Please fix CI, and you should update the e2e to include your new property, https://github.com/apache/skywalking-java/blob/main/test/e2e/case/expected/service-instance.yml

[nisiyong]

@wu-sheng The CI has been fixed.

[wu-sheng]

Error: Failed to execute goal org.springframework.boot:spring-boot-maven-plugin:3.0.0:repackage (default) on project undertow-worker-thread-pool-scenario: Execution default of goal org.springframework.boot:spring-boot-maven-plugin:3.0.0:repackage failed: Unable to load the mojo 'repackage' in the plugin 'org.springframework.boot:spring-boot-maven-plugin:3.0.0' due to an API incompatibility: org.codehaus.plexus.component.repository.exception.ComponentLookupException: org/springframework/boot/maven/RepackageMojo has been compiled by a more recent version of the Java Runtime (class file version 61.0), this version of the Java Runtime only recognizes class file versions up to 52.0

I think we need some fixes for some tests, which don't lock the Spring version. With the latest Sprinv boot 3.x released, it fails. Would you like to fix them in another PR first?

[nisiyong]

I think we need some fixes for some tests, which don't lock the Spring version. With the latest Sprinv boot 3.x released, it fails. Would you like to fix them in another PR first?

Okay, I will take a look.

[wu-sheng]

I am good with the idea and version format, version + commit ID.

Pending on @kezhenxu94 to review the codes.

[kezhenxu94]

lgtm

[wu-sheng]

@nisiyong Could you update the showcase repository to this commit? This could make our demo env show the version of the agent, at least for the Java agent

[nisiyong]

Sure, I will take some time to do it ASAP.