sonatype-lift[bot] commented on code in PR #9689: URL: https://github.com/apache/skywalking/pull/9689#discussion_r979843656
########## oap-server/server-storage-plugin/storage-shardingsphere-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/shardingsphere/ShardingRule.java: ########## @@ -0,0 +1,64 @@ +/* + * 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.oap.server.storage.plugin.jdbc.shardingsphere; + +import lombok.Builder; +import lombok.Getter; +import lombok.Setter; +import org.apache.skywalking.oap.server.storage.plugin.jdbc.SQLBuilder; + +@Getter +@Setter +@Builder(toBuilder = true) Review Comment: *[CanIgnoreReturnValueSuggester](https://errorprone.info/bugpattern/CanIgnoreReturnValueSuggester):* Methods that always 'return this' should be annotated with @CanIgnoreReturnValue --- ```suggestion @CanIgnoreReturnValue ``` --- <details><summary><b>âšī¸ Learn about @sonatype-lift commands</b></summary> You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings. | **Command** | **Usage** | | ------------- | ------------- | | `@sonatype-lift ignore` | Leave out the above finding from this PR | | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR | | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file | **Note:** When talking to LiftBot, you need to **refresh** the page to see its response. <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details> --- Was this a good recommendation? [ [đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=337681221&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=337681221&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681221&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681221&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=337681221&lift_comment_rating=5) ] ########## oap-server/server-storage-plugin/storage-shardingsphere-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/shardingsphere/mysql/MySQLShardingHistoryDeleteDAO.java: ########## @@ -0,0 +1,147 @@ +/* + * 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.oap.server.storage.plugin.jdbc.shardingsphere.mysql; + +import java.io.IOException; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.server.core.analysis.DownSampling; +import org.apache.skywalking.oap.server.core.storage.StorageException; +import org.apache.skywalking.oap.server.core.storage.model.Model; +import org.apache.skywalking.oap.server.library.client.jdbc.JDBCClientException; +import org.apache.skywalking.oap.server.library.client.jdbc.hikaricp.JDBCHikariCPClient; +import org.apache.skywalking.oap.server.library.module.ModuleManager; +import org.apache.skywalking.oap.server.storage.plugin.jdbc.h2.dao.H2HistoryDeleteDAO; +import org.joda.time.DateTime; + +@Slf4j +public class MySQLShardingHistoryDeleteDAO extends H2HistoryDeleteDAO { + + private final JDBCHikariCPClient client; + private final MySQLShardingStorageConfig config; + private final ModuleManager manager; + private final Set<String> dataSources; + private final Map<String, Long> tableLatestSuccess; + + public MySQLShardingHistoryDeleteDAO(JDBCHikariCPClient client, MySQLShardingStorageConfig config, + ModuleManager manager) { + super(client); + this.client = client; + this.config = config; + this.manager = manager; + this.dataSources = config.getDataSources(); + this.tableLatestSuccess = new HashMap<>(); + } + + @Override + public void deleteHistory(Model model, String timeBucketColumnName, int ttl) throws IOException { + if (!model.isRecord()) { + if (!DownSampling.Minute.equals(model.getDownsampling())) { + return; + } + } + + long deadline = Long.parseLong(new DateTime().plusDays(-ttl).toString("yyyyMMdd")); Review Comment: *[JodaConstructors](https://errorprone.info/bugpattern/JodaConstructors):* Use of certain JodaTime constructors are not allowed. --- ```suggestion long deadline = Long.parseLong(DateTime.now().plusDays(-ttl).toString("yyyyMMdd")); ``` --- <details><summary><b>âšī¸ Learn about @sonatype-lift commands</b></summary> You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings. | **Command** | **Usage** | | ------------- | ------------- | | `@sonatype-lift ignore` | Leave out the above finding from this PR | | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR | | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file | **Note:** When talking to LiftBot, you need to **refresh** the page to see its response. <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details> --- Was this a good recommendation? [ [đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=337681200&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=337681200&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681200&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681200&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=337681200&lift_comment_rating=5) ] ########## oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/manual/relation/instance/ServiceInstanceRelationServerSideMetrics.java: ########## @@ -27,17 +27,23 @@ import org.apache.skywalking.oap.server.core.analysis.worker.MetricsStreamProcessor; import org.apache.skywalking.oap.server.core.remote.grpc.proto.RemoteData; import org.apache.skywalking.oap.server.core.source.DefaultScopeDefine; +import org.apache.skywalking.oap.server.core.storage.ShardingAlgorithm; import org.apache.skywalking.oap.server.core.storage.annotation.BanyanDB; import org.apache.skywalking.oap.server.core.storage.annotation.Column; +import org.apache.skywalking.oap.server.core.storage.annotation.SQLDatabase; import org.apache.skywalking.oap.server.core.storage.type.Convert2Entity; import org.apache.skywalking.oap.server.core.storage.type.Convert2Storage; import org.apache.skywalking.oap.server.core.storage.type.StorageBuilder; +import static org.apache.skywalking.oap.server.core.analysis.metrics.Metrics.ENTITY_ID; +import static org.apache.skywalking.oap.server.core.analysis.metrics.Metrics.TIME_BUCKET; + @Stream(name = ServiceInstanceRelationServerSideMetrics.INDEX_NAME, scopeId = DefaultScopeDefine.SERVICE_INSTANCE_RELATION, builder = ServiceInstanceRelationServerSideMetrics.Builder.class, processor = MetricsStreamProcessor.class) @EqualsAndHashCode(of = { Review Comment: *[MissingOverride](https://errorprone.info/bugpattern/MissingOverride):* canEqual overrides method in Metrics; expected @Override --- ```suggestion @Override @EqualsAndHashCode(of = { ``` --- <details><summary><b>âšī¸ Learn about @sonatype-lift commands</b></summary> You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings. | **Command** | **Usage** | | ------------- | ------------- | | `@sonatype-lift ignore` | Leave out the above finding from this PR | | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR | | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file | **Note:** When talking to LiftBot, you need to **refresh** the page to see its response. <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details> --- Was this a good recommendation? [ [đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=337681195&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=337681195&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681195&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681195&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=337681195&lift_comment_rating=5) ] ########## oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBAggregationQueryDAO.java: ########## @@ -50,14 +50,14 @@ public BanyanDBAggregationQueryDAO(BanyanDBStorageClient client) { @Override public List<SelectedRecord> sortMetrics(TopNCondition condition, String valueColumnName, Duration duration, List<KeyValue> additionalConditions) throws IOException { Review Comment: *[MixedMutabilityReturnType](https://errorprone.info/bugpattern/MixedMutabilityReturnType):* This method returns both mutable and immutable collections or maps from different paths. This may be confusing for users of the method. --- ```suggestion return ImmutableList.of(); ``` --- <details><summary><b>âšī¸ Learn about @sonatype-lift commands</b></summary> You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings. | **Command** | **Usage** | | ------------- | ------------- | | `@sonatype-lift ignore` | Leave out the above finding from this PR | | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR | | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file | **Note:** When talking to LiftBot, you need to **refresh** the page to see its response. <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details> --- Was this a good recommendation? [ [đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=337681100&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=337681100&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681100&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681100&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=337681100&lift_comment_rating=5) ] ########## oap-server/server-storage-plugin/storage-shardingsphere-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/shardingsphere/mysql/MySQLShardingHistoryDeleteDAO.java: ########## @@ -0,0 +1,147 @@ +/* + * 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.oap.server.storage.plugin.jdbc.shardingsphere.mysql; + +import java.io.IOException; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.server.core.analysis.DownSampling; +import org.apache.skywalking.oap.server.core.storage.StorageException; +import org.apache.skywalking.oap.server.core.storage.model.Model; +import org.apache.skywalking.oap.server.library.client.jdbc.JDBCClientException; +import org.apache.skywalking.oap.server.library.client.jdbc.hikaricp.JDBCHikariCPClient; +import org.apache.skywalking.oap.server.library.module.ModuleManager; +import org.apache.skywalking.oap.server.storage.plugin.jdbc.h2.dao.H2HistoryDeleteDAO; +import org.joda.time.DateTime; + +@Slf4j +public class MySQLShardingHistoryDeleteDAO extends H2HistoryDeleteDAO { + + private final JDBCHikariCPClient client; + private final MySQLShardingStorageConfig config; + private final ModuleManager manager; + private final Set<String> dataSources; + private final Map<String, Long> tableLatestSuccess; + + public MySQLShardingHistoryDeleteDAO(JDBCHikariCPClient client, MySQLShardingStorageConfig config, + ModuleManager manager) { + super(client); + this.client = client; + this.config = config; + this.manager = manager; + this.dataSources = config.getDataSources(); + this.tableLatestSuccess = new HashMap<>(); + } + + @Override + public void deleteHistory(Model model, String timeBucketColumnName, int ttl) throws IOException { + if (!model.isRecord()) { + if (!DownSampling.Minute.equals(model.getDownsampling())) { + return; + } + } + + long deadline = Long.parseLong(new DateTime().plusDays(-ttl).toString("yyyyMMdd")); + //If it's a sharding table drop expired tables + if (model.getSqlDBModelExtension().isShardingTable()) { + boolean isRuleExecuted = false; + Long latestSuccessDeadline = this.tableLatestSuccess.get(model.getName()); + if (latestSuccessDeadline != null && deadline <= latestSuccessDeadline) { + if (log.isDebugEnabled()) { + log.debug("Table = {} already deleted, skip, deadline = {}, ttl = {}", model.getName(), deadline, ttl); + } + return; + } + try { + //refresh sharding rules + isRuleExecuted = ShardingRulesOperator.INSTANCE.createOrUpdateShardingRule(client, model, this.dataSources, ttl); + if (isRuleExecuted) { + MySQLShardingTableInstaller installer = new MySQLShardingTableInstaller(client, manager, config); + installer.createTable(model); + } + } catch (StorageException e) { + throw new IOException(e.getMessage(), e); + } + List<String> realTables = new ArrayList<>(); + try (Connection connection = client.getConnection()) { + ResultSet resultSet = connection.getMetaData() + .getTables(connection.getCatalog(), null, model.getName() + "_20%", null); + while (resultSet.next()) { + realTables.add(resultSet.getString("TABLE_NAME")); + } + + //delete additional tables + for (String additionalTable : model.getSqlDBModelExtension().getAdditionalTables().keySet()) { + ResultSet additionalTableRS = connection.getMetaData() + .getTables(connection.getCatalog(), null, + additionalTable + "_20%", null); + while (additionalTableRS.next()) { + realTables.add(additionalTableRS.getString("TABLE_NAME")); + } + } + } catch (JDBCClientException | SQLException e) { + throw new IOException(e.getMessage(), e); + } + + List<String> prepareDeleteTables = new ArrayList<>(); + for (String table : realTables) { + long timeSeries = isolateTimeFromTableName(table); + if (deadline >= timeSeries) { + prepareDeleteTables.add(table); + } + } + if (log.isDebugEnabled()) { + log.debug("Tables to be dropped: {}", prepareDeleteTables); Review Comment: *[CRLF_INJECTION_LOGS](https://find-sec-bugs.github.io/bugs.htm#CRLF_INJECTION_LOGS):* This use of org/slf4j/Logger.debug(Ljava/lang/String;Ljava/lang/Object;)V might be used to include CRLF characters into log messages --- <details><summary><b>âšī¸ Learn about @sonatype-lift commands</b></summary> You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings. | **Command** | **Usage** | | ------------- | ------------- | | `@sonatype-lift ignore` | Leave out the above finding from this PR | | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR | | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file | **Note:** When talking to LiftBot, you need to **refresh** the page to see its response. <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details> --- Was this a good recommendation? [ [đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=337681247&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=337681247&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681247&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681247&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=337681247&lift_comment_rating=5) ] ########## oap-server/server-storage-plugin/storage-shardingsphere-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/shardingsphere/mysql/MySQLShardingTableInstaller.java: ########## @@ -0,0 +1,157 @@ +/* + * 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.oap.server.storage.plugin.jdbc.shardingsphere.mysql; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.server.core.CoreModule; +import org.apache.skywalking.oap.server.core.UnexpectedException; +import org.apache.skywalking.oap.server.core.config.ConfigService; +import org.apache.skywalking.oap.server.core.storage.StorageException; +import org.apache.skywalking.oap.server.core.storage.model.Model; +import org.apache.skywalking.oap.server.core.storage.model.ModelColumn; +import org.apache.skywalking.oap.server.core.storage.model.SQLDatabaseExtension; +import org.apache.skywalking.oap.server.library.client.Client; +import org.apache.skywalking.oap.server.library.client.jdbc.JDBCClientException; +import org.apache.skywalking.oap.server.library.client.jdbc.hikaricp.JDBCHikariCPClient; +import org.apache.skywalking.oap.server.library.module.ModuleManager; +import org.apache.skywalking.oap.server.storage.plugin.jdbc.SQLBuilder; +import org.apache.skywalking.oap.server.storage.plugin.jdbc.TableMetaInfo; +import org.apache.skywalking.oap.server.storage.plugin.jdbc.mysql.MySQLTableInstaller; + +@Slf4j +public class MySQLShardingTableInstaller extends MySQLTableInstaller { + private final ConfigService configService; + private final Set<String> dataSources; + + public MySQLShardingTableInstaller(Client client, + ModuleManager moduleManager, + MySQLShardingStorageConfig config) { + super(client, moduleManager); + this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class); + this.dataSources = config.getDataSources(); + } + + @Override + protected boolean isExists(Model model) throws StorageException { + boolean isRuleExecuted = false; + boolean isTableExist = false; + if (configService == null) { + throw new UnexpectedException("ConfigService can not be null, should set ConfigService first."); + } + int ttl = model.isRecord() ? configService.getRecordDataTTL() : configService.getMetricsDataTTL(); + TableMetaInfo.addModel(model); + JDBCHikariCPClient h2Client = (JDBCHikariCPClient) client; + + try (Connection conn = h2Client.getConnection()) { + try (ResultSet rset = conn.getMetaData().getTables(conn.getCatalog(), null, model.getName(), null)) { + if (rset.next()) { + isTableExist = true; + } + } + } catch (SQLException | JDBCClientException e) { + throw new StorageException(e.getMessage(), e); + } + if (model.getSqlDBModelExtension().isShardingTable()) { + isRuleExecuted = ShardingRulesOperator.INSTANCE.createOrUpdateShardingRule(h2Client, model, this.dataSources, ttl); + } + return isTableExist && !isRuleExecuted; + } + + @Override + protected void createTable(Model model) throws StorageException { + //todo: init + super.createTable(model); + } + + @Override + protected void createTable(JDBCHikariCPClient client, + Connection connection, + String tableName, + List<ModelColumn> columns, + boolean additionalTable) throws JDBCClientException { + SQLBuilder tableCreateSQL = new SQLBuilder("CREATE TABLE IF NOT EXISTS " + tableName + " ("); + tableCreateSQL.append(ID_COLUMN).append(" VARCHAR(512) "); + if (!additionalTable) { + tableCreateSQL.appendLine("PRIMARY KEY, "); + } else { + tableCreateSQL.appendLine(", "); + } + for (int i = 0; i < columns.size(); i++) { + ModelColumn column = columns.get(i); + tableCreateSQL.appendLine( + getColumn(column) + (i != columns.size() ? "," : "")); + } + + int indexSeq = 0; + + //Add indexes + List<String> columnList = columns.stream().map(column -> column.getColumnName().getStorageName()).collect( + Collectors.toList()); + for (int i = 0; i < columns.size(); i++) { + ModelColumn column = columns.get(i); + for (final SQLDatabaseExtension.MultiColumnsIndex index : column.getSqlDatabaseExtension() + .getIndices()) { + final String[] multiColumns = index.getColumns(); + //Create MultiColumnsIndex on the additional table only when it contains all need columns. + if (additionalTable && !columnList.containsAll(Arrays.asList(multiColumns))) { + continue; + } + tableCreateSQL.append("KEY K") + .append(String.valueOf(indexSeq++)); + tableCreateSQL.append(" ("); + for (int j = 0; j < multiColumns.length; j++) { + tableCreateSQL.append(multiColumns[j]); + if (j < multiColumns.length - 1) { + tableCreateSQL.append(","); + } + } + tableCreateSQL.appendLine("),"); + } + if (column.shouldIndex() && column.getLength() < 513) { + tableCreateSQL.append("KEY K") + .append(String.valueOf(indexSeq++)); + tableCreateSQL.append(" (").append(column.getColumnName().getStorageName()).append(")"); + tableCreateSQL.appendLine(i != columns.size() - 1 ? "," : ""); + } + } + tableCreateSQL.appendLine(")"); + + if (log.isDebugEnabled()) { + log.debug("creating table: " + tableCreateSQL.toStringInNewLine()); + } + + client.execute(connection, tableCreateSQL.toString()); Review Comment: *[SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE](https://find-sec-bugs.github.io/bugs.htm#SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE):* org.apache.skywalking.oap.server.storage.plugin.jdbc.shardingsphere.mysql.MySQLShardingTableInstaller.createTable(JDBCHikariCPClient, Connection, String, List, boolean) passes a nonconstant String to an execute or addBatch method on an SQL statement --- <details><summary><b>âšī¸ Learn about @sonatype-lift commands</b></summary> You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings. | **Command** | **Usage** | | ------------- | ------------- | | `@sonatype-lift ignore` | Leave out the above finding from this PR | | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR | | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file | **Note:** When talking to LiftBot, you need to **refresh** the page to see its response. <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details> --- Was this a good recommendation? [ [đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=337681245&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=337681245&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681245&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681245&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=337681245&lift_comment_rating=5) ] ########## oap-server/server-storage-plugin/storage-shardingsphere-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/shardingsphere/mysql/ShardingRulesOperator.java: ########## @@ -0,0 +1,272 @@ +/* + * 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.oap.server.storage.plugin.jdbc.shardingsphere.mysql; + +import java.io.IOException; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.apache.skywalking.oap.server.core.UnexpectedException; +import org.apache.skywalking.oap.server.core.storage.ShardingAlgorithm; +import org.apache.skywalking.oap.server.core.storage.StorageException; +import org.apache.skywalking.oap.server.core.storage.model.Model; +import org.apache.skywalking.oap.server.core.storage.model.SQLDatabaseModelExtension; +import org.apache.skywalking.oap.server.library.client.jdbc.JDBCClientException; +import org.apache.skywalking.oap.server.library.client.jdbc.hikaricp.JDBCHikariCPClient; +import org.apache.skywalking.oap.server.storage.plugin.jdbc.SQLBuilder; +import org.apache.skywalking.oap.server.storage.plugin.jdbc.shardingsphere.ShardingRule; +import org.joda.time.DateTime; +import org.joda.time.Days; + +public enum ShardingRulesOperator { + INSTANCE; + + private final Map<String, ShardingRule> modelShardingRules = new HashMap<>(); + + private static final String TIME_RELATIVE_ID_SHARDING_EXPRESSION = + "${long time_bucket = Long.parseLong(id.substring(0,id.indexOf('_')));\n" + + "if (10000000L < time_bucket && time_bucket < 99999999L) {return time_bucket;}\n" + + "if (1000000000L < time_bucket && time_bucket < 9999999999L) {return time_bucket.intdiv(100);}\n" + + "if (100000000000L < time_bucket && time_bucket < 999999999999L) {return time_bucket.intdiv(100*100);}\n" + + "if (10000000000000L < time_bucket && time_bucket < 99999999999999L) {return time_bucket.intdiv(100*100*100);}\n" + + "}"; + + private static final String TIME_RANGE_SHARDING_EXPRESSION = + "\"datetime-pattern\"=\"yyyyMMddHHmmss\"," + + "\"datetime-interval-unit\"=\"days\"," + + "\"datetime-interval-amount\"=\"1\"," + + "\"sharding-suffix-pattern\"=\"yyyyMMdd\"," + + "\"datetime-lower\"=\"20220101000000\"," + + "\"datetime-upper\"=\"20991201000000\""; + + private static final String TIME_BUCKET_SHARDING_EXPRESSION = + "${\n" + + "if (10000000L < time_bucket && time_bucket < 99999999L) {return time_bucket;}\n" + + "if (1000000000L < time_bucket && time_bucket < 9999999999L) {return time_bucket.intdiv(100);}\n" + + "if (100000000000L < time_bucket && time_bucket < 999999999999L) {return time_bucket.intdiv(100*100);}\n" + + "if (10000000000000L < time_bucket && time_bucket < 99999999999999L) {return time_bucket.intdiv(100*100*100);}\n" + + "}"; + + public void start(JDBCHikariCPClient client) throws IOException, SQLException, StorageException { + initShardingRules(client); + } + + boolean createOrUpdateShardingRule(JDBCHikariCPClient client, Model model, Set<String> dataSources, int ttl) throws StorageException { + boolean isExecuted; + ShardingRule.ShardingRuleBuilder builder = ShardingRule.builder(); + builder.table(model.getName()); + String tableName = model.getName(); + SQLDatabaseModelExtension.Sharding sharding = model.getSqlDBModelExtension().getSharding(); + isExecuted = executeShardingRule( + buildShardingRule(builder, tableName, dataSources, sharding.getShardingAlgorithm(), + sharding.getTableShardingColumn(), + sharding.getDsShardingColumn(), + ttl, + new DateTime() + ), + client, + tableName + ); + // additional tables + for (String additionalTable : model.getSqlDBModelExtension().getAdditionalTables().keySet()) { + ShardingRule.ShardingRuleBuilder additionalBuilder = ShardingRule.builder(); + additionalBuilder.table(additionalTable); + isExecuted = executeShardingRule( + buildShardingRule(additionalBuilder, additionalTable, dataSources, sharding.getShardingAlgorithm(), + sharding.getTableShardingColumn(), + sharding.getDsShardingColumn(), ttl, new DateTime() + ), client, additionalTable + ); + } + + return isExecuted; + } + + private void initShardingRules(JDBCHikariCPClient client) throws IOException { + SQLBuilder sql = new SQLBuilder("SHOW SHARDING TABLE RULES"); + try (Connection connection = client.getConnection()) { + ResultSet resultSet = client.executeQuery(connection, sql.toString()); + while (resultSet.next()) { + ShardingRule.ShardingRuleBuilder builder = ShardingRule.builder(); + builder.table(resultSet.getString("TABLE")); + builder.actualDataNodes(resultSet.getString("ACTUAL_DATA_NODES")); + builder.actualDataSources(resultSet.getString("ACTUAL_DATA_SOURCES")); + // todo: shardingsphere-5.1.2 response DATABASE_STRATEGY and TABLE_STRATEGY type "inline" but should "standard" + builder.databaseStrategyType("standard"); + builder.databaseShardingColumn(resultSet.getString("DATABASE_SHARDING_COLUMN")); + builder.databaseShardingAlgorithmType(resultSet.getString("DATABASE_SHARDING_ALGORITHM_TYPE")); + builder.databaseShardingAlgorithmProps(resultSet.getString("DATABASE_SHARDING_ALGORITHM_PROPS")); + // todo: shardingsphere-5.1.2 response DATABASE_STRATEGY and TABLE_STRATEGY type "inline" but should "standard" + builder.tableStrategyType("standard"); + builder.tableShardingColumn(resultSet.getString("TABLE_SHARDING_COLUMN")); + builder.tableShardingAlgorithmType(resultSet.getString("TABLE_SHARDING_ALGORITHM_TYPE")); + builder.tableShardingAlgorithmProps(resultSet.getString("TABLE_SHARDING_ALGORITHM_PROPS")); + builder.keyGenerateColumn(resultSet.getString("KEY_GENERATE_COLUMN")); + builder.keyGeneratorType(resultSet.getString("KEY_GENERATOR_TYPE")); + builder.keyGeneratorProps(resultSet.getString("KEY_GENERATOR_PROPS")); + + ShardingRule shardingRule = builder.build(); + modelShardingRules.put(shardingRule.getTable(), shardingRule); + } + } catch (JDBCClientException | SQLException e) { + throw new IOException(e.getMessage(), e); + } + } + + private void registerShardingRule(String tableName, ShardingRule rule) { + modelShardingRules.put(tableName, rule); + } + + private ShardingRule.ShardingRuleBuilder buildShardingRule(ShardingRule.ShardingRuleBuilder builder, + String tableName, + Set<String> dataSources, + ShardingAlgorithm shardingAlgorithm, + String tableShardingColumn, + String dsShardingColumn, + int ttl, + DateTime currentDate) { + buildDataNodes(builder, tableName, dataSources, ttl, currentDate); + buildDatabaseStrategy(builder, dsShardingColumn, dataSources.size()); + + switch (shardingAlgorithm) { + case TIME_RANGE_SHARDING_ALGORITHM: + buildTimeRangeTableStrategy(builder, tableShardingColumn); + break; + case TIME_RELATIVE_ID_SHARDING_ALGORITHM: + buildExpressionTableStrategy(builder, tableName, tableShardingColumn, + TIME_RELATIVE_ID_SHARDING_EXPRESSION); + break; + case TIME_BUCKET_SHARDING_ALGORITHM: + buildExpressionTableStrategy(builder, tableName, tableShardingColumn, TIME_BUCKET_SHARDING_EXPRESSION); + break; + default: + throw new UnexpectedException("Unsupported sharding algorithm " + shardingAlgorithm); + } + return builder; + } + + private void buildDataNodes(ShardingRule.ShardingRuleBuilder builder, + String tableName, + Set<String> dataSources, + int ttl, + DateTime currentDate) { + DateTime startDateTime = currentDate.plusDays(1 - ttl); + DateTime endDate = currentDate.plusDays(1); + int days = Days.daysBetween(startDateTime, endDate).getDays(); + List<DateTime> timeRanges = new ArrayList<>(); + StringBuilder nodesBuilder = new StringBuilder(); + for (int i = 0; i <= days; i++) { + timeRanges.add(startDateTime.plusDays(i)); + } + dataSources.forEach(dataSource -> { + timeRanges.forEach(dateTime -> { + nodesBuilder.append("\"") + .append(dataSource) + .append(".") + .append(tableName) + .append("_") + .append(dateTime.toString("yyyyMMdd")) + .append("\"") + .append(","); + }); + }); + builder.actualDataNodes(nodesBuilder.substring(0, nodesBuilder.length() - 1)); + } + + private void buildDatabaseStrategy(ShardingRule.ShardingRuleBuilder builder, + String shardingColumn, + int dsCount) { + StringBuilder propsBuilder = new StringBuilder(); + propsBuilder.append("\"algorithm-expression\"=\"ds_${") + .append(shardingColumn) + .append(".hashCode()&Integer.MAX_VALUE%") + .append(dsCount) + .append("}\""); + + builder.databaseStrategyType("\"standard\"") + .databaseShardingColumn(shardingColumn) + .databaseShardingAlgorithmType("\"inline\"") + .databaseShardingAlgorithmProps(propsBuilder.toString()); + + } + + private void buildTimeRangeTableStrategy(ShardingRule.ShardingRuleBuilder builder, + String tableShardingColumn) { + builder.tableStrategyType("\"standard\"") + .tableShardingColumn(tableShardingColumn) + .tableShardingAlgorithmType("\"interval\"") + .tableShardingAlgorithmProps(TIME_RANGE_SHARDING_EXPRESSION); + } + + private void buildExpressionTableStrategy(ShardingRule.ShardingRuleBuilder builder, + String tableName, + String tableShardingColumn, + String algorithmExpression + ) { + StringBuilder propsBuilder = new StringBuilder(); + propsBuilder.append("\"allow-range-query-with-inline-sharding\"=\"true\",") + .append("\"algorithm-expression\"=\"") + .append(tableName) + .append("_") + .append(algorithmExpression) + .append("\""); + + builder.tableStrategyType("\"standard\"") + .tableShardingColumn(tableShardingColumn) + .tableShardingAlgorithmType("\"inline\"") + .tableShardingAlgorithmProps(propsBuilder.toString()); + + } + + private boolean executeShardingRule(ShardingRule.ShardingRuleBuilder builder, + JDBCHikariCPClient client, + String tableName) throws StorageException { + ShardingRule existRule = modelShardingRules.get(tableName); + ShardingRule shardingRule; + String shardingRuleSQL; + if (existRule == null) { + builder.operation("CREATE"); + shardingRule = builder.build(); + shardingRuleSQL = shardingRule.toShardingRuleSQL(); + } else { + builder.operation("ALTER"); + existRule.setOperation("ALTER"); + shardingRule = builder.build(); + shardingRuleSQL = shardingRule.toShardingRuleSQL(); + if (existRule.toShardingRuleSQL().equals(shardingRuleSQL.replaceAll("\"", ""))) { Review Comment: *[UNSAFE_HASH_EQUALS](https://find-sec-bugs.github.io/bugs.htm#UNSAFE_HASH_EQUALS):* Unsafe comparison of hash that are susceptible to timing attack --- <details><summary><b>âšī¸ Learn about @sonatype-lift commands</b></summary> You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings. | **Command** | **Usage** | | ------------- | ------------- | | `@sonatype-lift ignore` | Leave out the above finding from this PR | | `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR | | `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file | **Note:** When talking to LiftBot, you need to **refresh** the page to see its response. <sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details> --- Was this a good recommendation? [ [đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=337681253&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=337681253&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681253&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=337681253&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=337681253&lift_comment_rating=5) ] -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
