You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/11/24 04:10:11 UTC
[GitHub] [skywalking-java] nisiyong opened a new pull request, #397: Report agent version to OAP
nisiyong opened a new pull request, #397:
URL: https://github.com/apache/skywalking-java/pull/397
<!--
β οΈ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
### Report 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. I use [git-commit-id-maven-plugin](https://github.com/git-commit-id/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 #<issue number>.
- [ ] Update the [`CHANGES` log](https://github.com/apache/skywalking-java/blob/main/CHANGES.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328271646
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1330025525
@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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] kezhenxu94 commented on pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1325945149
> > 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
https://github.com/apache/skywalking-java/blob/70b3e88b0fc133d4e74dbe54ae945d01b860a735/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/remote/AgentIDDecorator.java#L41-L42
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on a diff in pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on code in PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#discussion_r1031711909
##########
apm-sniffer/apm-agent/pom.xml:
##########
@@ -52,6 +52,31 @@
<build>
<finalName>skywalking-agent</finalName>
<plugins>
+ <plugin>
+ <groupId>pl.project13.maven</groupId>
+ <artifactId>git-commit-id-plugin</artifactId>
+ <version>4.0.5</version>
+ <executions>
+ <execution>
+ <id>get-the-git-infos</id>
+ <goals>
+ <goal>revision</goal>
+ </goals>
+ <phase>initialize</phase>
+ </execution>
+ </executions>
+ <configuration>
+ <generateGitPropertiesFile>true</generateGitPropertiesFile>
+ <failOnNoGitDirectory>false</failOnNoGitDirectory>
Review Comment:
@wu-sheng @kezhenxu94 This property won't let the source code without the `.git` dir build failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1326040194
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1326060854
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328180461
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328268759
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328268860
> I think buildVersion + commitId is enough for me
This works for me too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1325945779
@kezhenxu94 OKay, thanks. I take a look first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on a diff in pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on code in PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#discussion_r1031710629
##########
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/util/InstanceJsonPropertiesUtil.java:
##########
@@ -45,7 +55,32 @@ public static List<KeyStringValuePair> parseProperties() {
properties.add(KeyStringValuePair.newBuilder().setKey("namespace").setValue(Config.Agent.NAMESPACE).build());
properties.add(KeyStringValuePair.newBuilder().setKey("cluster").setValue(Config.Agent.CLUSTER).build());
+ properties.add(KeyStringValuePair.newBuilder().setKey("version").setValue(getAgentVersion()).build());
return properties;
}
+
+ public static String getAgentVersion() {
+ try {
+ InputStream inStream = InstanceJsonPropertiesUtil.class.getClassLoader()
+ .getResourceAsStream(GIT_PROPERTIES);
+ if (inStream != null) {
+ Properties gitProperties = new Properties();
+ gitProperties.load(inStream);
+ String commitIdAbbrev = gitProperties.getProperty("git.commit.id.abbrev");
+ String buildTime = gitProperties.getProperty("git.build.time");
+ String buildVersion = gitProperties.getProperty("git.build.version");
+ String version = buildVersion + "-" + commitIdAbbrev + "-" + buildTime;
+
+ LOGGER.info("SkyWalking agent version: {}", version);
+ return version;
+ } else {
+ LOGGER.warn("{} not found, SkyWalking agent version: unknown", GIT_PROPERTIES);
Review Comment:
@kezhenxu94 If users build from the source tarball, here is the WARN level logging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1325934049
> 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] kezhenxu94 commented on pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1325939838
I remember we have already reported the agent version to OAP long time ago, although we didn't persist the version into storage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on a diff in pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on code in PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#discussion_r1032803631
##########
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/version/Version.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.agent.core.version;
+
+import lombok.Getter;
+
+import java.io.IOException;
+import java.util.Properties;
+
+@Getter
+public enum Version {
+ CURRENT;
+
+ private static final String VERSION_FILE_NAME = "skywalking-agent-version.properties";
+ private final String buildVersion;
+ private final String commitIdAbbrev;
+ private final String buildTime;
Review Comment:
It works well in my local. Here is the log after I run the script `create_release.sh` with my personal repo. You can see the properties contains the field`buildTime`.
```
β skywalking-java git:(report-agent-version) β ./tools/releasing/create_release.sh
Release version 8.14.0-test
Source tag v8.14.0-test
Creating source package
Cloning into './apache-skywalking-java-agent-8.14.0-test'...
remote: Enumerating objects: 253586, done.
remote: Counting objects: 100% (1700/1700), done.
remote: Compressing objects: 100% (683/683), done.
remote: Total 253586 (delta 593), reused 1523 (delta 466), pack-reused 251886
Receiving objects: 100% (253586/253586), 158.63 MiB | 17.31 MiB/s, done.
Resolving deltas: 100% (98366/98366), done.
Submodule 'apm-protocol/apm-network/src/main/proto' (https://github.com/apache/skywalking-data-collect-protocol.git) registered for path 'apm-protocol/apm-network/src/main/proto'
Cloning into '/Users/nisiyong/CodeProjects/skywalking-java/apache-skywalking-java-agent-8.14.0-test/apm-protocol/apm-network/src/main/proto'...
Submodule path 'apm-protocol/apm-network/src/main/proto': checked out 'f9066463deb7f9d1fbe8110eab3524694b0db298'
--2022-11-26 23:48:11-- https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.5/maven-wrapper-0.5.5.jar
Resolving repo.maven.apache.org (repo.maven.apache.org)... 151.101.40.215
Connecting to repo.maven.apache.org (repo.maven.apache.org)|151.101.40.215|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 50710 (50K) [application/java-archive]
Saving to: β/Users/nisiyong/CodeProjects/skywalking-java/apache-skywalking-java-agent-8.14.0-test/.mvn/wrapper/maven-wrapper.jarβ
/Users/nisiyong/CodeProjects/skywalking-java/apache-skywalking-java-agent-8. 100%[============================================================================================================================================================================================>] 49.52K 257KB/s in 0.2s
2022-11-26 23:48:12 (257 KB/s) - β/Users/nisiyong/CodeProjects/skywalking-java/apache-skywalking-java-agent-8.14.0-test/.mvn/wrapper/maven-wrapper.jarβ saved [50710/50710]
β skywalking-java git:(report-agent-version) β find apache-skywalking-java-agent-8.14.0-test -name "*version.properties"
apache-skywalking-java-agent-8.14.0-test/apm-sniffer/apm-agent-core/src/main/resources/skywalking-agent-version.properties
β skywalking-java git:(report-agent-version) β cat apache-skywalking-java-agent-8.14.0-test/apm-sniffer/apm-agent-core/src/main/resources/skywalking-agent-version.properties
#Generated by Git-Commit-Id-Plugin
git.build.time=221126
git.build.version=8.14.0-SNAPSHOT
git.commit.id.abbrev=2ef1977
git.commit.id.full=2ef19778aeea34364d0e8643b093981963a17962
β skywalking-java git:(report-agent-version) β
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328176105
> 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 https://github.com/git-commit-id/git-commit-id-maven-plugin/pull/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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328266912
I want to keep both scenarios having the same version style. Because in the ASF, only the source tar build is the real release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1329143059
> 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328136687
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng merged pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng merged PR #397:
URL: https://github.com/apache/skywalking-java/pull/397
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1325990705
@wu-sheng @kezhenxu94
After reading [AgentIDDecorator.java](https://github.com/apache/skywalking-java/blob/70b3e88b0fc133d4e74dbe54ae945d01b860a735/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/remote/AgentIDDecorator.java#L41-L42), 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] kezhenxu94 commented on a diff in pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#discussion_r1032720927
##########
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/version/Version.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.agent.core.version;
+
+import lombok.Getter;
+
+import java.io.IOException;
+import java.util.Properties;
+
+@Getter
+public enum Version {
+ CURRENT;
+
+ private static final String VERSION_FILE_NAME = "skywalking-agent-version.properties";
+ private final String buildVersion;
+ private final String commitIdAbbrev;
+ private final String buildTime;
Review Comment:
The build time doesn't work as expected and might bring confusion, can you confirm? See https://github.com/apache/skywalking/pull/8934
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] kezhenxu94 commented on a diff in pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#discussion_r1032855714
##########
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/version/Version.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.agent.core.version;
+
+import lombok.Getter;
+
+import java.io.IOException;
+import java.util.Properties;
+
+@Getter
+public enum Version {
+ CURRENT;
+
+ private static final String VERSION_FILE_NAME = "skywalking-agent-version.properties";
+ private final String buildVersion;
+ private final String commitIdAbbrev;
+ private final String buildTime;
Review Comment:
Please read the link I pasted and the [related issue](https://github.com/apache/skywalking/issues/8887), I meant the build time doesn't work **as expected**, we expect the build time to change every time you build, but the build time just stays at the moment when you first build.
In your log, `git.build.time=221126` is correct yesterday, but doesn't change some time in the future
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on a diff in pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on code in PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#discussion_r1032944861
##########
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/version/Version.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.agent.core.version;
+
+import lombok.Getter;
+
+import java.io.IOException;
+import java.util.Properties;
+
+@Getter
+public enum Version {
+ CURRENT;
+
+ private static final String VERSION_FILE_NAME = "skywalking-agent-version.properties";
+ private final String buildVersion;
+ private final String commitIdAbbrev;
+ private final String buildTime;
Review Comment:
I have tested it, and you are right. But the build time will be changed every time within the `.git` scenario if we run `mvn clean package`, because the properties file was generated in the `target` output directory which was cleaned.
In the source tall scenario, the properties file was generated in the resource directory, so we couldn't overwrite the build time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328268129
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1329137321
@wu-sheng The CI has been fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1330026923
Sure, I will take some time to do it ASAP.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328071202
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
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328266242
> 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
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1326904845
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1326936157
Okay, I will find the solution for source tarball recently.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1325942578
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1326943474
@kezhenxu94 Thanks, I will take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1325944886
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] kezhenxu94 commented on pull request #397: Report agent version to OAP
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1326027368
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] kezhenxu94 commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1326942445
> Okay, I will find the solution for source tarball recently.
If you have no better idea, take a look at how OAP did this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] nisiyong commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
nisiyong commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1329147490
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #397: Report the agent version to OAP
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #397:
URL: https://github.com/apache/skywalking-java/pull/397#issuecomment-1328272753
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org