diff --git a/changelog/unreleased/000057-connection-tags-fix.yml b/changelog/unreleased/000057-connection-tags-fix.yml new file mode 100644 index 0000000..c53faa8 --- /dev/null +++ b/changelog/unreleased/000057-connection-tags-fix.yml @@ -0,0 +1,10 @@ +title: Fixed an issue with registering hofund.node, caused by inconsistent sets of tag keys across different connection types +authors: + - name: Mateusz Witkowski + nick: witx98 + url: https://github.com/witx98 +type: fixed #[added/changed/deprecated/removed/fixed/security/other] +issues: + - 57 +merge_requests: + - 58 \ No newline at end of file diff --git a/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundConnection.java b/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundConnection.java index 54888cf..f8fb1c5 100644 --- a/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundConnection.java +++ b/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundConnection.java @@ -1,5 +1,6 @@ package dev.logchange.hofund.connection; +import dev.logchange.hofund.StringUtils; import lombok.Builder; import lombok.Data; @@ -8,6 +9,7 @@ @Data @Builder public class HofundConnection { + /** * Name of the resource that application connects to f.e. cart-db, fcm, products-app */ @@ -15,5 +17,13 @@ public class HofundConnection { private final String url; private final Type type; private final AtomicReference fun; + private final String description; + + public String toTargetTag() { + return type == Type.DATABASE ? target + "_" + type : target; + } + public String getDescription() { + return StringUtils.emptyIfNull(description); + } } diff --git a/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundConnectionMeter.java b/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundConnectionMeter.java index 3b3b459..9ff1046 100644 --- a/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundConnectionMeter.java +++ b/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundConnectionMeter.java @@ -27,30 +27,21 @@ public HofundConnectionMeter(HofundInfoProvider infoProvider, List tags(HofundInfoProvider infoProvider, HofundConnection connection) { + private List tags(HofundConnection connection) { List tags = new LinkedList<>(); tags.add(Tag.of("id", infoProvider.getApplicationName() + "-" + connection.getTarget() + "_" + connection.getType())); tags.add(Tag.of("source", infoProvider.getApplicationName())); - - String target = connection.getTarget(); - if (connection.getType() == Type.DATABASE) { - target += "_" + connection.getType(); - tags.add(Tag.of("db_vendor", ((HofundDatabaseConnection) connection).getDbVendor())); - } - - tags.add(Tag.of("target", target)); + tags.add(Tag.of("target", connection.toTargetTag())); tags.add(Tag.of("type", connection.getType().toString())); - + tags.add(Tag.of("description", connection.getDescription())); return tags; } @Override public void bindTo(MeterRegistry meterRegistry) { - connections.forEach(connection -> { - Gauge.builder(NAME, connection, con -> con.getFun().get().getStatus().getValue()) - .description(DESCRIPTION) - .tags(tags(infoProvider, connection)) - .register(meterRegistry); - }); + connections.forEach(connection -> Gauge.builder(NAME, connection, con -> con.getFun().get().getStatus().getValue()) + .description(DESCRIPTION) + .tags(tags(connection)) + .register(meterRegistry)); } } diff --git a/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundDatabaseConnection.java b/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundDatabaseConnection.java deleted file mode 100644 index df68234..0000000 --- a/hofund-core/src/main/java/dev/logchange/hofund/connection/HofundDatabaseConnection.java +++ /dev/null @@ -1,21 +0,0 @@ -package dev.logchange.hofund.connection; - -import lombok.Builder; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.ToString; - -import java.util.concurrent.atomic.AtomicReference; - -@Getter -@ToString(callSuper = true) -@EqualsAndHashCode(callSuper = true) -public class HofundDatabaseConnection extends HofundConnection { - private final String dbVendor; - - @Builder(builderMethodName = "hofundDatabaseConnectionBuilder") - public HofundDatabaseConnection(String target, String url, Type type, AtomicReference fun, String dbVendor) { - super(target, url, type, fun); - this.dbVendor = dbVendor; - } -} diff --git a/hofund-core/src/main/java/dev/logchange/hofund/graph/edge/HofundEdgeMeter.java b/hofund-core/src/main/java/dev/logchange/hofund/graph/edge/HofundEdgeMeter.java index 8860b92..6d688e2 100644 --- a/hofund-core/src/main/java/dev/logchange/hofund/graph/edge/HofundEdgeMeter.java +++ b/hofund-core/src/main/java/dev/logchange/hofund/graph/edge/HofundEdgeMeter.java @@ -1,14 +1,15 @@ package dev.logchange.hofund.graph.edge; import dev.logchange.hofund.connection.HofundConnection; -import dev.logchange.hofund.connection.HofundConnectionMeter; import dev.logchange.hofund.connection.HofundConnectionsProvider; import dev.logchange.hofund.info.HofundInfoProvider; import io.micrometer.core.instrument.Gauge; import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.binder.MeterBinder; import java.util.Collection; +import java.util.LinkedList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; @@ -37,12 +38,19 @@ public HofundEdgeMeter(HofundInfoProvider infoProvider, List { - Gauge.builder(NAME, atomicInteger, AtomicInteger::doubleValue) - .description(DESCRIPTION) - .tags(HofundConnectionMeter.tags(infoProvider, connection)) - .register(meterRegistry); - }); + connections.forEach(connection -> Gauge.builder(NAME, atomicInteger, AtomicInteger::doubleValue) + .description(DESCRIPTION) + .tags(tags(connection)) + .register(meterRegistry)); } + + private List tags(HofundConnection connection) { + List tags = new LinkedList<>(); + tags.add(Tag.of("id", infoProvider.getApplicationName() + "-" + connection.getTarget() + "_" + connection.getType())); + tags.add(Tag.of("source", infoProvider.getApplicationName())); + tags.add(Tag.of("target", connection.toTargetTag())); + tags.add(Tag.of("type", connection.getType().toString())); + return tags; + } } diff --git a/hofund-core/src/main/java/dev/logchange/hofund/graph/node/HofundNodeMeter.java b/hofund-core/src/main/java/dev/logchange/hofund/graph/node/HofundNodeMeter.java index 413efb2..ee5442f 100644 --- a/hofund-core/src/main/java/dev/logchange/hofund/graph/node/HofundNodeMeter.java +++ b/hofund-core/src/main/java/dev/logchange/hofund/graph/node/HofundNodeMeter.java @@ -2,8 +2,6 @@ import dev.logchange.hofund.connection.HofundConnection; import dev.logchange.hofund.connection.HofundConnectionsProvider; -import dev.logchange.hofund.connection.HofundDatabaseConnection; -import dev.logchange.hofund.connection.Type; import dev.logchange.hofund.info.HofundInfoProvider; import io.micrometer.core.instrument.Gauge; import io.micrometer.core.instrument.MeterRegistry; @@ -44,12 +42,10 @@ public void bindTo(MeterRegistry meterRegistry) { .tags(tagsForInfo()) .register(meterRegistry); - connections.forEach(connection -> { - Gauge.builder(NAME, atomicInteger, AtomicInteger::doubleValue) - .description(DESCRIPTION) - .tags(tagsForConnection(connection)) - .register(meterRegistry); - }); + connections.forEach(connection -> Gauge.builder(NAME, atomicInteger, AtomicInteger::doubleValue) + .description(DESCRIPTION) + .tags(tagsForConnection(connection)) + .register(meterRegistry)); } @@ -66,18 +62,10 @@ private List tagsForInfo() { private List tagsForConnection(HofundConnection connection) { List tags = new LinkedList<>(); - - String target = connection.getTarget(); - if (connection.getType() == Type.DATABASE) { - target += "_" + connection.getType(); - tags.add(Tag.of("db_vendor", ((HofundDatabaseConnection) connection).getDbVendor())); - } - - tags.add(Tag.of("id", target)); + tags.add(Tag.of("id", connection.toTargetTag())); tags.add(Tag.of("title", connection.getTarget() + "_" + connection.getType())); - tags.add(Tag.of("subtitle", connection.getType().toString())); + tags.add(Tag.of("subtitle", connection.getType().toString() + " (" + connection.getDescription() + ")")); tags.add(Tag.of("type", connection.getType().toString())); - return tags; } } diff --git a/hofund-core/src/test/java/dev/logchange/hofund/connection/HofundConnectionTest.java b/hofund-core/src/test/java/dev/logchange/hofund/connection/HofundConnectionTest.java new file mode 100644 index 0000000..c1fcb6c --- /dev/null +++ b/hofund-core/src/test/java/dev/logchange/hofund/connection/HofundConnectionTest.java @@ -0,0 +1,73 @@ +package dev.logchange.hofund.connection; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class HofundConnectionTest { + + + @ParameterizedTest + @EnumSource( + value = Type.class, + names = "DATABASE", + mode = EnumSource.Mode.EXCLUDE) + void shouldTargetPropertyWhenTypeIsDifferentThanDB() { + // given: + HofundConnection connection = HofundConnection.builder() + .target("Target") + .type(Type.HTTP) + .build(); + + // when: + String targetTag = connection.toTargetTag(); + + // then: + assertEquals("Target", targetTag); + } + + @Test + void shouldTargetPropertyWhenTypeIsDB() { + // given: + HofundConnection connection = HofundConnection.builder() + .target("Target") + .type(Type.DATABASE) + .build(); + + String expected = "Target_database"; + + // when: + String targetTag = connection.toTargetTag(); + + // then: + assertEquals(expected, targetTag); + } + + @Test + void shouldReturnEmptyDescriptionWhenPropertyIsNull() { + // given: + HofundConnection connection = HofundConnection.builder().build(); + + // when: + String description = connection.getDescription(); + + // then: + assertEquals("", description); + } + + @Test + void shouldReturnValueOfDescriptionWhenPropertyIsNotNull() { + // given: + + String expected = "Description"; + HofundConnection connection = HofundConnection.builder().description(expected).build(); + + // when: + String description = connection.getDescription(); + + // then: + assertEquals(expected, description); + } +} diff --git a/hofund-core/src/test/java/dev/logchange/hofund/connection/HofundConnectionsTableTest.java b/hofund-core/src/test/java/dev/logchange/hofund/connection/HofundConnectionsTableTest.java index 2c06067..ab01f79 100644 --- a/hofund-core/src/test/java/dev/logchange/hofund/connection/HofundConnectionsTableTest.java +++ b/hofund-core/src/test/java/dev/logchange/hofund/connection/HofundConnectionsTableTest.java @@ -79,7 +79,8 @@ private static HofundConnection getHofundConnection(String target, Type type) { target, "fake", type, - new AtomicReference<>(() -> Status.UP) + new AtomicReference<>(() -> Status.UP), + null ); } } \ No newline at end of file diff --git a/hofund-spring/src/main/java/dev/logchange/hofund/connection/spring/datasource/DataSourceConnectionFactory.java b/hofund-spring/src/main/java/dev/logchange/hofund/connection/spring/datasource/DataSourceConnectionFactory.java index 6f067e9..3163964 100644 --- a/hofund-spring/src/main/java/dev/logchange/hofund/connection/spring/datasource/DataSourceConnectionFactory.java +++ b/hofund-spring/src/main/java/dev/logchange/hofund/connection/spring/datasource/DataSourceConnectionFactory.java @@ -1,6 +1,6 @@ package dev.logchange.hofund.connection.spring.datasource; -import dev.logchange.hofund.connection.HofundDatabaseConnection; +import dev.logchange.hofund.connection.HofundConnection; import dev.logchange.hofund.connection.spring.datasource.h2.H2Connection; import dev.logchange.hofund.connection.spring.datasource.oracle.OracleConnection; import dev.logchange.hofund.connection.spring.datasource.postgresql.PostgreSQLConnection; @@ -14,7 +14,7 @@ @Slf4j public class DataSourceConnectionFactory { - public static HofundDatabaseConnection of(DataSource dataSource) { + public static HofundConnection of(DataSource dataSource) { try (Connection connection = dataSource.getConnection()) { DatabaseMetaData metaData = connection.getMetaData(); String productName = metaData.getDatabaseProductName(); @@ -30,7 +30,7 @@ public static HofundDatabaseConnection of(DataSource dataSource) { } }; } catch (SQLException e) { - log.warn("Error creating HofundDatabaseConnection", e); + log.warn("Error creating datasource HofundConnection", e); return null; } } diff --git a/hofund-spring/src/main/java/dev/logchange/hofund/connection/spring/datasource/DatasourceConnection.java b/hofund-spring/src/main/java/dev/logchange/hofund/connection/spring/datasource/DatasourceConnection.java index 26042d4..e1b0edf 100644 --- a/hofund-spring/src/main/java/dev/logchange/hofund/connection/spring/datasource/DatasourceConnection.java +++ b/hofund-spring/src/main/java/dev/logchange/hofund/connection/spring/datasource/DatasourceConnection.java @@ -1,11 +1,17 @@ package dev.logchange.hofund.connection.spring.datasource; -import dev.logchange.hofund.connection.*; +import dev.logchange.hofund.connection.HofundConnection; +import dev.logchange.hofund.connection.Status; +import dev.logchange.hofund.connection.StatusFunction; +import dev.logchange.hofund.connection.Type; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import javax.sql.DataSource; -import java.sql.*; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; import java.util.Objects; import java.util.concurrent.atomic.AtomicReference; @@ -17,11 +23,11 @@ public abstract class DatasourceConnection { protected final DataSource dataSource; protected final String testQuery; - public HofundDatabaseConnection toHofundConnection() { - return HofundDatabaseConnection.hofundDatabaseConnectionBuilder() + public HofundConnection toHofundConnection() { + return HofundConnection.builder() .target(getTarget()) .type(Type.DATABASE) - .dbVendor(getDbVendor()) + .description(getDbVendor()) .url(getUrl()) .fun(new AtomicReference<>(testConnection())) .build();