Pochatkin commented on code in PR #3485: URL: https://github.com/apache/ignite-3/pull/3485#discussion_r1541124456
########## modules/eventlog/src/test/java/org/apache/ignite/internal/eventlog/sink/LogSinkTest.java: ########## @@ -0,0 +1,114 @@ +/* + * 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.ignite.internal.eventlog.sink; + +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; +import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; +import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; +import org.apache.ignite.internal.eventlog.api.Event; +import org.apache.ignite.internal.eventlog.config.schema.EventLogConfiguration; +import org.apache.ignite.internal.eventlog.config.schema.LogSinkChange; +import org.apache.ignite.internal.eventlog.event.EventUser; +import org.apache.ignite.internal.eventlog.event.IgniteEvents; +import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(ConfigurationExtension.class) +@Disabled("https://issues.apache.org/jira/browse/IGNITE-21850";) +class LogSinkTest extends BaseIgniteAbstractTest { + + @InjectConfiguration + private EventLogConfiguration cfg; + + private static File eventlogFile; Review Comment: Use java.nio.file.Path ########## modules/eventlog/src/test/java/org/apache/ignite/internal/eventlog/sink/LogSinkTest.java: ########## @@ -0,0 +1,114 @@ +/* + * 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.ignite.internal.eventlog.sink; + +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; +import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; +import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; +import org.apache.ignite.internal.eventlog.api.Event; +import org.apache.ignite.internal.eventlog.config.schema.EventLogConfiguration; +import org.apache.ignite.internal.eventlog.config.schema.LogSinkChange; +import org.apache.ignite.internal.eventlog.event.EventUser; +import org.apache.ignite.internal.eventlog.event.IgniteEvents; +import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(ConfigurationExtension.class) +@Disabled("https://issues.apache.org/jira/browse/IGNITE-21850";) +class LogSinkTest extends BaseIgniteAbstractTest { + + @InjectConfiguration + private EventLogConfiguration cfg; + + private static File eventlogFile; + + @BeforeAll + static void beforeAll() { + String buildDirPath = System.getProperty("buildDirPath"); Review Comment: Why not working dir? ########## modules/eventlog/src/main/java/org/apache/ignite/internal/eventlog/sink/LogSink.java: ########## @@ -0,0 +1,52 @@ +/* + * 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.ignite.internal.eventlog.sink; + +import java.lang.System.Logger; +import java.lang.System.Logger.Level; +import org.apache.ignite.internal.eventlog.api.Event; +import org.apache.ignite.internal.eventlog.config.schema.LogSinkView; +import org.apache.ignite.internal.eventlog.ser.EventSerializer; +import org.apache.ignite.internal.eventlog.ser.JsonEventSerializer; +import org.apache.ignite.internal.lang.IgniteInternalException; +import org.apache.ignite.lang.ErrorGroups.Common; + +/** Sink that writes events to the log using any logging framework the user has configured. */ +public class LogSink implements Sink { + private final Logger logger; + private final EventSerializer serializer; + private final String level; + private final String format; + + LogSink(LogSinkView cfg) { + this.level = cfg.level(); + this.format = cfg.format(); + this.logger = System.getLogger(cfg.criteria()); + this.serializer = new JsonEventSerializer(); + } + + /** {@inheritDoc} */ + @Override + public void write(Event event) { + if (!"json".equalsIgnoreCase(format)) { Review Comment: Do we really need this check here? We have validator in configuration which guarantee that this condition is true. What the reason for this second check? ########## modules/eventlog/src/test/java/org/apache/ignite/internal/eventlog/sink/LogSinkTest.java: ########## @@ -0,0 +1,114 @@ +/* + * 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.ignite.internal.eventlog.sink; + +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; +import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; +import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; +import org.apache.ignite.internal.eventlog.api.Event; +import org.apache.ignite.internal.eventlog.config.schema.EventLogConfiguration; +import org.apache.ignite.internal.eventlog.config.schema.LogSinkChange; +import org.apache.ignite.internal.eventlog.event.EventUser; +import org.apache.ignite.internal.eventlog.event.IgniteEvents; +import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(ConfigurationExtension.class) +@Disabled("https://issues.apache.org/jira/browse/IGNITE-21850";) +class LogSinkTest extends BaseIgniteAbstractTest { + + @InjectConfiguration + private EventLogConfiguration cfg; + + private static File eventlogFile; + + @BeforeAll + static void beforeAll() { + String buildDirPath = System.getProperty("buildDirPath"); + eventlogFile = Path.of(buildDirPath).resolve("event.log").toFile(); + } + + @AfterAll + static void afterAll() { + if (eventlogFile.exists()) { Review Comment: Files.deleteIfExists() ########## modules/eventlog/src/main/java/org/apache/ignite/internal/eventlog/event/EventImpl.java: ########## @@ -20,10 +20,13 @@ import java.util.Map; import java.util.Objects; import org.apache.ignite.internal.eventlog.api.Event; +import org.apache.ignite.internal.eventlog.ser.JsonEventSerializer; /** * Implementation of the {@link Event} interface. The class is immutable and thread-safe. * If you want to create an instance of this class, use the {@link EventBuilder}. + * Review Comment: Do we have tests for this Note? ########## modules/eventlog/src/main/java/org/apache/ignite/internal/eventlog/sink/Sink.java: ########## @@ -0,0 +1,42 @@ +/* + * 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.ignite.internal.eventlog.sink; + +import org.apache.ignite.internal.eventlog.api.Event; + +/** + * The endpoint for the event log framework. This is the last step in the event log pipeline. + * It can be a log file, a webhook, or a Kafka topic, or whatever we develop. + * + * <p>The contract of the only method is the following: + * + * <p>IT DOES NOT GUARANTEE THAT THE EVENT IS WRITTEN TO THE FINAL DESTINATION. Review Comment: I would say that this is implementation details. Better to write here that implementation not must satisfy of this contract. ########## modules/eventlog/src/main/java/org/apache/ignite/internal/eventlog/sink/SinkFactory.java: ########## @@ -0,0 +1,37 @@ +/* + * 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.ignite.internal.eventlog.sink; + +import org.apache.ignite.internal.eventlog.config.schema.LogSinkView; +import org.apache.ignite.internal.eventlog.config.schema.SinkView; + +/** + * Factory for creating sink instances. + */ +public class SinkFactory { + /** + * Creates a sink instance. + * + * @param sinkView Sink configuration view. + * @return Sink instance. + */ + public Sink createSink(SinkView sinkView) { + // Now only LogSink is supported. + return new LogSink((LogSinkView) sinkView); Review Comment: Better to add instance of operator here. And throw exception in case of another View type. Just for future. ########## modules/eventlog/src/test/java/org/apache/ignite/internal/eventlog/sink/LogSinkTest.java: ########## @@ -0,0 +1,114 @@ +/* + * 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.ignite.internal.eventlog.sink; + +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; +import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; +import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; +import org.apache.ignite.internal.eventlog.api.Event; +import org.apache.ignite.internal.eventlog.config.schema.EventLogConfiguration; +import org.apache.ignite.internal.eventlog.config.schema.LogSinkChange; +import org.apache.ignite.internal.eventlog.event.EventUser; +import org.apache.ignite.internal.eventlog.event.IgniteEvents; +import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(ConfigurationExtension.class) +@Disabled("https://issues.apache.org/jira/browse/IGNITE-21850";) +class LogSinkTest extends BaseIgniteAbstractTest { + + @InjectConfiguration + private EventLogConfiguration cfg; + + private static File eventlogFile; + + @BeforeAll + static void beforeAll() { + String buildDirPath = System.getProperty("buildDirPath"); + eventlogFile = Path.of(buildDirPath).resolve("event.log").toFile(); + } + + @AfterAll + static void afterAll() { + if (eventlogFile.exists()) { + eventlogFile.delete(); + } + } + + @Test + void logsToFile() throws Exception { + // Given log sink configuration. + cfg.change(c -> c.changeSinks().create("logSink", s -> { + LogSinkChange logSinkChange = (LogSinkChange) s.convert("log"); + logSinkChange.changeCriteria("EventLog"); + logSinkChange.changeLevel("INFO"); + logSinkChange.changeFormat("json"); + })).get(); + // And log sink. + Sink logSink = new SinkFactory().createSink(cfg.sinks().get("logSink").value()); + // And event. + Event event = IgniteEvents.USER_AUTHENTICATED.create( + EventUser.of("user1", "basicProvider") + ); + + // When write event into log sink. + logSink.write(event); + + // Then event is written to file. + await().untilAsserted(() -> assertThat(readLines(eventlogFile), hasSize(1))); + // And event is written in JSON format. + var expectedEventJson = "{" + + "\"type\":\"USER_AUTHENTICATED\"," + + "\"timestamp\":" + event.timestamp() + "," + + "\"productVersion\":\"" + event.productVersion() + "\"," + + "\"user\":{\"username\":\"user1\",\"authenticationProvider\":\"basicProvider\"}," + + "\"fields\":{}" + + "}"; + assertThat(readLines(eventlogFile), hasItem(expectedEventJson)); + + // When write one more event. + Event event2 = IgniteEvents.CONNECTION_CLOSED.create( + EventUser.of("user2", "basicProvider") + ); + + logSink.write(event2); + + // Then both events are written to file. + await().untilAsserted(() -> assertThat(readLines(eventlogFile), hasSize(2))); + } + + private static List<String> readLines(File file) throws IOException { Review Comment: Replace with Files.readAllLines -- 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]
