Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
ivan-penchev commented on code in PR #1234:
URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1666321239
##
pulsar/log/wrapper_slog_test.go:
##
@@ -0,0 +1,49 @@
+// 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.
+
+//go:build go1.21
+
+package log
+
+import (
+ "bytes"
+ "encoding/json"
+ "log/slog"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestSlog(t *testing.T) {
Review Comment:
I tried to do it now.
Can you run the CI pipeline?
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
ivan-penchev commented on code in PR #1234:
URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1666321239
##
pulsar/log/wrapper_slog_test.go:
##
@@ -0,0 +1,49 @@
+// 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.
+
+//go:build go1.21
+
+package log
+
+import (
+ "bytes"
+ "encoding/json"
+ "log/slog"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestSlog(t *testing.T) {
Review Comment:
I tried to do it now.
Can you run the CI pipeline
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
ivan-penchev commented on code in PR #1234:
URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1666321239
##
pulsar/log/wrapper_slog_test.go:
##
@@ -0,0 +1,49 @@
+// 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.
+
+//go:build go1.21
+
+package log
+
+import (
+ "bytes"
+ "encoding/json"
+ "log/slog"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestSlog(t *testing.T) {
Review Comment:
I tried to do it now.
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
ivan-penchev commented on code in PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1666286324 ## README.md: ## @@ -152,7 +152,7 @@ Run the tests: Run the tests with specific versions of GOLANG and PULSAR: -make test GOLANG_VERSION=1.20 PULSAR_VERSION=2.10.0 +make test GO_VERSION=1.20 PULSAR_VERSION=2.10.0 Review Comment: I changed it to match with the name of the variable inside the Makefile.  -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
ivan-penchev commented on code in PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1666286324 ## README.md: ## @@ -152,7 +152,7 @@ Run the tests: Run the tests with specific versions of GOLANG and PULSAR: -make test GOLANG_VERSION=1.20 PULSAR_VERSION=2.10.0 +make test GO_VERSION=1.20 PULSAR_VERSION=2.10.0 Review Comment: Did not match with the name of the variable inside the Makefile.  -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on code in PR #1234:
URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1666194036
##
pulsar/log/wrapper_slog_test.go:
##
@@ -0,0 +1,49 @@
+// 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.
+
+//go:build go1.21
+
+package log
+
+import (
+ "bytes"
+ "encoding/json"
+ "log/slog"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestSlog(t *testing.T) {
Review Comment:
Could you test all levels and prints? This can help the library provide a
helpful log.
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on code in PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1666190182 ## README.md: ## @@ -152,7 +152,7 @@ Run the tests: Run the tests with specific versions of GOLANG and PULSAR: -make test GOLANG_VERSION=1.20 PULSAR_VERSION=2.10.0 +make test GO_VERSION=1.20 PULSAR_VERSION=2.10.0 Review Comment: Why change this name? -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix][broker] Fix MessageDeduplication replay timeout cause topic loading stuck [pulsar]
TakaHiR07 commented on code in PR #23004:
URL: https://github.com/apache/pulsar/pull/23004#discussion_r1666176443
##
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/TopicDuplicationTest.java:
##
@@ -529,6 +538,89 @@ public void
testDisableNamespacePolicyTakeSnapshotShouldNotThrowException() thro
persistentTopic.checkDeduplicationSnapshot();
}
+@Test
+public void testFinishTakeSnapshotWhenTopicLoading() throws Exception {
Review Comment:
Test has been improved. Currently would not passed
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][broker] Fix MessageDeduplication replay timeout cause topic loading stuck [pulsar]
TakaHiR07 commented on code in PR #23004:
URL: https://github.com/apache/pulsar/pull/23004#discussion_r1666175402
##
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/TopicDuplicationTest.java:
##
@@ -529,6 +538,88 @@ public void
testDisableNamespacePolicyTakeSnapshotShouldNotThrowException() thro
persistentTopic.checkDeduplicationSnapshot();
}
+@Test
+public void testFinishTakeSnapshotWhenTopicLoading() throws Exception {
+cleanup();
+setup();
+
+// Create a topic and wait deduplication is started.
+int brokerDeduplicationEntriesInterval = 1000;
+pulsar.getConfiguration().setBrokerDeduplicationEnabled(true);
+
pulsar.getConfiguration().setBrokerDeduplicationEntriesInterval(brokerDeduplicationEntriesInterval);
+final String topic =
BrokerTestUtil.newUniqueName("persistent://public/default/tp");
+admin.topics().createNonPartitionedTopic(topic);
+final PersistentTopic persistentTopic1 =
+(PersistentTopic) pulsar.getBrokerService().getTopic(topic,
false).join().get();
+final ManagedLedgerImpl ml1 = (ManagedLedgerImpl)
persistentTopic1.getManagedLedger();
+Awaitility.await().untilAsserted(() -> {
+ManagedCursorImpl cursor1 =
+(ManagedCursorImpl)
ml1.getCursors().get(PersistentTopic.DEDUPLICATION_CURSOR_NAME);
+assertNotNull(cursor1);
+});
+final MessageDeduplication deduplication1 =
persistentTopic1.getMessageDeduplication();
+
+
+// Send 999 messages, it is less than
"brokerDeduplicationEntriesInterval".
+// So it would not trigger takeSnapshot
+final Producer producer =
pulsarClient.newProducer(Schema.STRING)
+.topic(topic).enableBatching(false).create();
+for (int i = 0; i < brokerDeduplicationEntriesInterval - 1; i++) {
+producer.send(i + "");
+}
+producer.close();
+int snapshotCounter1 = WhiteboxImpl.getInternalState(deduplication1,
"snapshotCounter");
+assertEquals(snapshotCounter1, brokerDeduplicationEntriesInterval - 1);
+
+
+// Unload and load topic, simulate topic load is timeout.
+// SetBrokerDeduplicationEntriesInterval to 10, therefore
recoverSequenceIdsMap#takeSnapshot
+// would trigger and should update the snapshot position.
+// However, if topic close and takeSnapshot are concurrent,
+// it would result in takeSnapshot throw exception
+admin.topics().unload(topic);
+pulsar.getConfiguration().setBrokerDeduplicationEntriesInterval(10);
+
+Field field2 = BrokerService.class.getDeclaredField("topics");
+field2.setAccessible(true);
+ConcurrentOpenHashMap>>
topics =
+(ConcurrentOpenHashMap>>)
+field2.get(pulsar.getBrokerService());
+
+pulsar.getBrokerService().getTopic(topic, false);
+Assert.assertTrue(topics.containsKey(topic));
+CompletableFuture> future = topics.get(topic);
+future.completeExceptionally(FutureUtil.createTimeoutException("Failed
to load topic within timeout",
Review Comment:
That's helpful. Thx. Have updated the test
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][broker] The topic might reference a closed ledger [pulsar]
TakaHiR07 commented on PR #22860: URL: https://github.com/apache/pulsar/pull/22860#issuecomment-2209878165 > 1. When the broker loads a topic and times it out, it should not complete the future to the client before closing all resources. This way seems better. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix][broker] Fix MessageDeduplication replay timeout cause topic loading stuck [pulsar]
shibd commented on code in PR #23004:
URL: https://github.com/apache/pulsar/pull/23004#discussion_r1666137904
##
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/TopicDuplicationTest.java:
##
@@ -529,6 +538,88 @@ public void
testDisableNamespacePolicyTakeSnapshotShouldNotThrowException() thro
persistentTopic.checkDeduplicationSnapshot();
}
+@Test
+public void testFinishTakeSnapshotWhenTopicLoading() throws Exception {
+cleanup();
+setup();
+
+// Create a topic and wait deduplication is started.
+int brokerDeduplicationEntriesInterval = 1000;
+pulsar.getConfiguration().setBrokerDeduplicationEnabled(true);
+
pulsar.getConfiguration().setBrokerDeduplicationEntriesInterval(brokerDeduplicationEntriesInterval);
+final String topic =
BrokerTestUtil.newUniqueName("persistent://public/default/tp");
+admin.topics().createNonPartitionedTopic(topic);
+final PersistentTopic persistentTopic1 =
+(PersistentTopic) pulsar.getBrokerService().getTopic(topic,
false).join().get();
+final ManagedLedgerImpl ml1 = (ManagedLedgerImpl)
persistentTopic1.getManagedLedger();
+Awaitility.await().untilAsserted(() -> {
+ManagedCursorImpl cursor1 =
+(ManagedCursorImpl)
ml1.getCursors().get(PersistentTopic.DEDUPLICATION_CURSOR_NAME);
+assertNotNull(cursor1);
+});
+final MessageDeduplication deduplication1 =
persistentTopic1.getMessageDeduplication();
+
+
+// Send 999 messages, it is less than
"brokerDeduplicationEntriesInterval".
+// So it would not trigger takeSnapshot
+final Producer producer =
pulsarClient.newProducer(Schema.STRING)
+.topic(topic).enableBatching(false).create();
+for (int i = 0; i < brokerDeduplicationEntriesInterval - 1; i++) {
+producer.send(i + "");
+}
+producer.close();
+int snapshotCounter1 = WhiteboxImpl.getInternalState(deduplication1,
"snapshotCounter");
+assertEquals(snapshotCounter1, brokerDeduplicationEntriesInterval - 1);
+
+
+// Unload and load topic, simulate topic load is timeout.
+// SetBrokerDeduplicationEntriesInterval to 10, therefore
recoverSequenceIdsMap#takeSnapshot
+// would trigger and should update the snapshot position.
+// However, if topic close and takeSnapshot are concurrent,
+// it would result in takeSnapshot throw exception
+admin.topics().unload(topic);
+pulsar.getConfiguration().setBrokerDeduplicationEntriesInterval(10);
+
+Field field2 = BrokerService.class.getDeclaredField("topics");
+field2.setAccessible(true);
+ConcurrentOpenHashMap>>
topics =
+(ConcurrentOpenHashMap>>)
+field2.get(pulsar.getBrokerService());
+
+pulsar.getBrokerService().getTopic(topic, false);
+Assert.assertTrue(topics.containsKey(topic));
+CompletableFuture> future = topics.get(topic);
+future.completeExceptionally(FutureUtil.createTimeoutException("Failed
to load topic within timeout",
Review Comment:
https://github.com/apache/pulsar/blob/a91a172b4ee6d8b974a3fa905e435975557fcc57/pulsar-broker/src/test/java/org/apache/pulsar/client/api/OrphanPersistentTopicTest.java#L127-L137
Maybe we can refer to these code to mock topic load timeout and
deduplication take snapshot timeout.
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [I] [Bug][broker] MessageDeduplication replay timeout would cause topic loading stuck and become unavailable [pulsar]
shibd commented on issue #23003: URL: https://github.com/apache/pulsar/issues/23003#issuecomment-2209762114 > The reason is if topic loading timeout, the topic would close. However, topic close and takeSnapshot is executed concurrently, so takeSnapshot may throw exception since topic has been closed. Maybe we need add stats: (closing, closed) to avoid this concurrently behaviour. https://github.com/apache/pulsar/blob/411f6973e85b0a6213e992386e1704f93d0aae42/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/MessageDeduplication.java#L61-L80 By the way, when your service encounters this exception, does it recover automatically? Can you try to reproduce this behavior in this test? testCloseLedgerThatTopicAfterCreateTimeout -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix][broker] The topic might reference a closed ledger [pulsar]
shibd commented on PR #22860: URL: https://github.com/apache/pulsar/pull/22860#issuecomment-2209737237 > client can acquire the timeout response immediately and retry. Yes, you are right. Maybe we need improve client, when get a timoutexception, don't be so aggressive in retrying. or, we should completely refactor the broker's behavior in topic load: 1. When the broker loads a topic and times it out, it should not complete the future to the client before closing all resources. Anyway, the current solution will not leave the topic in an unusable state. (It is link with a closed ledger.) WHDYT? -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [feat][misc] PIP-264: Copy OpenTelemetry resource attributes to Prometheus labels [pulsar]
dragosvictor opened a new pull request, #23005: URL: https://github.com/apache/pulsar/pull/23005 [PIP-264](https://github.com/apache/pulsar/blob/master/pip/pip-264.md) ### Motivation The OpenTelemetry collector can be configured to expose the metrics as a Prometheus server, simplifying the deployment and migration of the telemetry pipeline. Pulsar supports this feature out of the box. There is a catch though: the Prometheus exporter did not copy the OpenTelemetry resource attributes over to corresponding Prometheus labels, so vital information was lost in this process. This limitation is described in https://github.com/open-telemetry/opentelemetry-java/issues/6108, which has since been fixed. The present PR leverages this improvement, instructing the OpenTelemetry SDK to migrate _all_ resource attributes over to Prometheus as labels. ### Modifications - During OpenTelemetry SDK initialization in `OpenTelemetryService`, configure any Prometheus exporters to copy all resource attributes as labels. Caveat: At the time this happens, the Prometheus server is already initialized and listening. Reconfiguring it on-the-go is not possible, so the server is closed and recreated with the desired configuration. ### Verifying this change - [ ] Make sure that the change passes the CI checks. This change modified tests and can be verified as follows: - Updated integration test `OpenTelemetrySanityTest#testOpenTelemetryMetricsPrometheusExport` to validate the labels are attached to the metrics exposed by the Prometheus exporter. ### Does this pull request potentially affect one of the following parts: - [ ] Dependencies (add or upgrade a dependency) - [ ] The public API - [ ] The schema - [ ] The default values of configurations - [ ] The threading model - [ ] The binary protocol - [ ] The REST endpoints - [ ] The admin CLI options - [x] The metrics: _As described above_ - [ ] Anything that affects deployment ### Documentation - [ ] `doc` - [ ] `doc-required` https://github.com/apache/pulsar-site/pull/929 - [x] `doc-not-needed` - [ ] `doc-complete` ### Matching PR in forked repository PR in forked repository: https://github.com/dragosvictor/pulsar/pull/38 -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on PR #1234:
URL:
https://github.com/apache/pulsar-client-go/pull/1234#issuecomment-2209260330
> 1. Where should I place the test?
You can write a test in the `pulsar/client_impl_with_slog_test.go`.
> 2. What should I be testing? (that producer and consumer do not throw
error when using Slog?)
All right.
> 3. How do I ensure my test is run only when we build for with go 1.21
```go
/// 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.
//go:build go1.21
package pulsar
import "testing"
func TestClientWithSlog(t *testing.T) {
}
```
> For 2 you could see - in my PR description, I did an integration test
where I started a producer with logrus, and producer with slog - and then I
compared the output of both loggers, in terms of length and keys (since I was
doing json). But this seems too complicated and not needed for what I was
trying to test.
>
> So I would be provocative and ask, do we even need a test?
You only need to verify the client with slog works fine.
You can also verify the log output:
```go
// 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.
//go:build go1.21
package log
import (
"context"
"log/slog"
"testing"
"github.com/stretchr/testify/assert"
)
type defaultHandle struct {
records []slog.Record
}
func (d *defaultHandle) Enabled(ctx context.Context, level slog.Level) bool {
return true
}
func (d *defaultHandle) Handle(ctx context.Context, record slog.Record)
error {
d.records = append(d.records, record)
return nil
}
func (d *defaultHandle) WithAttrs(attrs []slog.Attr) slog.Handler {
panic("implement me")
}
func (d *defaultHandle) WithGroup(name string) slog.Handler {
panic("implement me")
}
var _ slog.Handler = {}
func TestSlog(t *testing.T) {
handler := {}
logger := slog.New(handler)
logger.Info("1")
assert.Equal(t, handler.records[0].Level, slog.LevelInfo)
assert.Equal(t, handler.records[0].Message, "1")
}
```
Thanks for your 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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][client] Fix pattern consumer create crash if a part of partitions of a topic have been deleted [pulsar]
poorbarcode commented on code in PR #22854:
URL: https://github.com/apache/pulsar/pull/22854#discussion_r1665801286
##
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java:
##
@@ -585,19 +585,28 @@ private CompletableFuture>
patternTopicSubscribeAsync(ConsumerCo
lookup.getTopicsUnderNamespace(namespaceName, subscriptionMode, regex,
null)
.thenAccept(getTopicsResult -> {
if (log.isDebugEnabled()) {
-log.debug("Get topics under namespace {}, topics.size: {},"
-+ " topicsHash: {}, changed: {}, filtered:
{}",
+log.debug("Pattern consumer [{}] get topics under
namespace {}, topics.size: {},"
++ " topicsHash: {}, changed: {}, filtered:
{}", conf.getSubscriptionName(),
namespaceName, getTopicsResult.getTopics().size(),
getTopicsResult.getTopicsHash(),
getTopicsResult.isChanged(),
getTopicsResult.isFiltered());
getTopicsResult.getTopics().forEach(topicName ->
-log.debug("Get topics under namespace {}, topic: {}",
namespaceName, topicName));
+log.debug("Pattern consumer [{}] get topics under
namespace {}, topic: {}",
+conf.getSubscriptionName(), namespaceName,
topicName));
}
List topicsList = getTopicsResult.getTopics();
if (!getTopicsResult.isFiltered()) {
topicsList =
TopicList.filterTopics(getTopicsResult.getTopics(), pattern);
}
conf.getTopicNames().addAll(topicsList);
+
+if (log.isInfoEnabled()) {
Review Comment:
Changed all logs that related to topics changes to `Debug`, thanks
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][client] Fix pattern consumer create crash if a part of partitions of a topic have been deleted [pulsar]
poorbarcode commented on code in PR #22854:
URL: https://github.com/apache/pulsar/pull/22854#discussion_r1665797519
##
pulsar-broker/src/test/java/org/apache/pulsar/client/impl/LookupServiceTest.java:
##
@@ -108,8 +108,8 @@ public void testGetTopicsOfGetTopicsResult(boolean
isUsingHttpLookup) throws Exc
// Verify the new method
"GetTopicsResult.nonPartitionedOrPartitionTopics" works as expected.
Collection nonPartitionedOrPartitionTopics =
lookupService.getTopicsUnderNamespace(NamespaceName.get("public/default"),
Review Comment:
Sorry, it is a mistake. removed this change. Thanks
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][client] Fix pattern consumer create crash if a part of partitions of a topic have been deleted [pulsar]
mattisonchao commented on code in PR #22854:
URL: https://github.com/apache/pulsar/pull/22854#discussion_r1665793987
##
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java:
##
@@ -585,19 +585,28 @@ private CompletableFuture>
patternTopicSubscribeAsync(ConsumerCo
lookup.getTopicsUnderNamespace(namespaceName, subscriptionMode, regex,
null)
.thenAccept(getTopicsResult -> {
if (log.isDebugEnabled()) {
-log.debug("Get topics under namespace {}, topics.size: {},"
-+ " topicsHash: {}, changed: {}, filtered:
{}",
+log.debug("Pattern consumer [{}] get topics under
namespace {}, topics.size: {},"
++ " topicsHash: {}, changed: {}, filtered:
{}", conf.getSubscriptionName(),
namespaceName, getTopicsResult.getTopics().size(),
getTopicsResult.getTopicsHash(),
getTopicsResult.isChanged(),
getTopicsResult.isFiltered());
getTopicsResult.getTopics().forEach(topicName ->
-log.debug("Get topics under namespace {}, topic: {}",
namespaceName, topicName));
+log.debug("Pattern consumer [{}] get topics under
namespace {}, topic: {}",
+conf.getSubscriptionName(), namespaceName,
topicName));
}
List topicsList = getTopicsResult.getTopics();
if (!getTopicsResult.isFiltered()) {
topicsList =
TopicList.filterTopics(getTopicsResult.getTopics(), pattern);
}
conf.getTopicNames().addAll(topicsList);
+
+if (log.isInfoEnabled()) {
Review Comment:
It might make sense to use the debug level.
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][client] Fix pattern consumer create crash if a part of partitions of a topic have been deleted [pulsar]
mattisonchao commented on code in PR #22854:
URL: https://github.com/apache/pulsar/pull/22854#discussion_r1665793987
##
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java:
##
@@ -585,19 +585,28 @@ private CompletableFuture>
patternTopicSubscribeAsync(ConsumerCo
lookup.getTopicsUnderNamespace(namespaceName, subscriptionMode, regex,
null)
.thenAccept(getTopicsResult -> {
if (log.isDebugEnabled()) {
-log.debug("Get topics under namespace {}, topics.size: {},"
-+ " topicsHash: {}, changed: {}, filtered:
{}",
+log.debug("Pattern consumer [{}] get topics under
namespace {}, topics.size: {},"
++ " topicsHash: {}, changed: {}, filtered:
{}", conf.getSubscriptionName(),
namespaceName, getTopicsResult.getTopics().size(),
getTopicsResult.getTopicsHash(),
getTopicsResult.isChanged(),
getTopicsResult.isFiltered());
getTopicsResult.getTopics().forEach(topicName ->
-log.debug("Get topics under namespace {}, topic: {}",
namespaceName, topicName));
+log.debug("Pattern consumer [{}] get topics under
namespace {}, topic: {}",
+conf.getSubscriptionName(), namespaceName,
topicName));
}
List topicsList = getTopicsResult.getTopics();
if (!getTopicsResult.isFiltered()) {
topicsList =
TopicList.filterTopics(getTopicsResult.getTopics(), pattern);
}
conf.getTopicNames().addAll(topicsList);
+
+if (log.isInfoEnabled()) {
Review Comment:
It might be debug level is making sense.
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][client] Fix pattern consumer create crash if a part of partitions of a topic have been deleted [pulsar]
mattisonchao commented on code in PR #22854:
URL: https://github.com/apache/pulsar/pull/22854#discussion_r1665785500
##
pulsar-broker/src/test/java/org/apache/pulsar/client/impl/LookupServiceTest.java:
##
@@ -108,8 +108,8 @@ public void testGetTopicsOfGetTopicsResult(boolean
isUsingHttpLookup) throws Exc
// Verify the new method
"GetTopicsResult.nonPartitionedOrPartitionTopics" works as expected.
Collection nonPartitionedOrPartitionTopics =
lookupService.getTopicsUnderNamespace(NamespaceName.get("public/default"),
Review Comment:
Why do we need to change 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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][broker] Fix MessageDeduplication replay timeout cause topic loading stuck [pulsar]
BewareMyPower commented on code in PR #23004:
URL: https://github.com/apache/pulsar/pull/23004#discussion_r1665659571
##
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/TopicDuplicationTest.java:
##
@@ -529,6 +538,89 @@ public void
testDisableNamespacePolicyTakeSnapshotShouldNotThrowException() thro
persistentTopic.checkDeduplicationSnapshot();
}
+@Test
+public void testFinishTakeSnapshotWhenTopicLoading() throws Exception {
Review Comment:
This test passed locally even without changes of `MessageDeduplication`,
could you explain it?
https://github.com/apache/pulsar/assets/18204803/ffe988f0-575a-4b2f-aa52-d961bd78ad8e;>
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] [fix][broker] Fix MessageDeduplication replay timeout cause topic loading stuck [pulsar]
TakaHiR07 opened a new pull request, #23004: URL: https://github.com/apache/pulsar/pull/23004 Fixes https://github.com/apache/pulsar/issues/23003 ### Motivation As shown in the issue. fix the risk of topic loading stuck. ### Modifications 1. make takeSnapshot() async , and if topic loading timeout, topic close should wait until takeSnapshot finish. 2. add test for this issue scene ### Verifying this change - [x] Make sure that the change passes the CI checks. ### Documentation - [ ] `doc` - [ ] `doc-required` - [x] `doc-not-needed` - [ ] `doc-complete` -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] [Bug][broker] MessageDeduplication replay timeout would cause topic loading stuck and become unavailable [pulsar]
TakaHiR07 opened a new issue, #23003: URL: https://github.com/apache/pulsar/issues/23003 ### Search before asking - [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar. ### Read release policy - [X] I understand that unsupported versions don't get bug fixes. I will attempt to reproduce the issue on a supported version of Pulsar client and Pulsar broker. ### Version master ### Minimal reproduce step enter this dead loop, and topic loading keep failed. 1. topic load 2. MessageDeduplication replay need much time 3. topic load timeout 4. topic close and reload ### What did you expect to see? topic become available ### What did you see instead? The issue's root is as following: https://github.com/apache/pulsar/pull/21540 , this pr modify that topic would be closed if 60s timeout. https://github.com/apache/pulsar/pull/22479, this pr add a logic that takeSnapshot after MessageDeduplication replay, so that topic loading won't timeout. https://github.com/apache/pulsar/pull/22860, this pr refactor the topic loading process. Now topic loading should not be concurrent. If topic loading would timeout, the loading process is sequentially "create -> close -> create". However, topic loading is still stuck. The reason is if topic loading timeout, the topic would close. However, topic close and takeSnapshot is executed concurrently, so takeSnapshot may throw exception since topic has been closed. This would result in each time we retry loading topic, we need to replaying the same entries in MessageDeduplication, and we are always 60s timeout. The error log is : ``` 17:49:30.300 [broker-topic-workers-OrderedExecutor-6-0] INFO org.apache.pulsar.broker.service.persistent.MessageDeduplication - [persistent://test/test/test-partition-0] Replaying 2383098 entries for deduplication 17:53:05.845 [BookKeeperClientWorker-OrderedExecutor-17-0] INFO org.apache.pulsar.broker.service.persistent.MessageDeduplication - [persistent://test/test/test-partition-0] Enabled deduplication 17:53:05.886 [bookkeeper-ml-scheduler-OrderedScheduler-5-0] WARN org.apache.bookkeeper.mledger.impl.ManagedCursorImpl - [test/test/persistent/test-partition-0] Failed to update cursor metadata for pulsar.dedup due to version conflict org.apache.pulsar.metadata.api.MetadataStoreException$BadVersionException: org.apache.zookeeper.KeeperException$BadVersionException: KeeperErrorCode = BadVersion for /managed-ledgers/test/test/persistent/test-partition-0/pulsar.dedup 17:53:05.908 [BookKeeperClientWorker-OrderedExecutor-6-0] WARN org.apache.pulsar.broker.service.persistent.MessageDeduplication - [persistent://test/test/test-partition-0] Failed to store new deduplication snapshot at 1033467:11005. ex: org.apache.bookkeeper.mledger.ManagedLedgerException$BadVersionException: org.apache.pulsar.metadata.api.MetadataStoreException$BadVersionException: org.apache.zookeeper.KeeperException$BadVersionException: KeeperErrorCode = BadVersion for /managed-ledgers/test/test/persistent/test-partition-0/pulsar.dedup 17:53:05.974 [bookkeeper-ml-scheduler-OrderedScheduler-5-0] INFO org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test-partition-0] Recovery for cursor pulsar.dedup completed. pos=1028289:2816218 -- todo=0 17:53:05.975 [broker-topic-workers-OrderedExecutor-3-0] INFO org.apache.pulsar.broker.service.persistent.MessageDeduplication - [persistent://test/test/test-partition-0] Replaying 2383098 entries for deduplication 17:56:06.764 [BookKeeperClientWorker-OrderedExecutor-19-0] WARN org.apache.pulsar.broker.service.persistent.MessageDeduplication - [persistent://test/test/test-partition-0] Failed to store new deduplication snapshot at 1033467:11005. ex: org.apache.bookkeeper.mledger.ManagedLedgerException$MetaStoreException: org.apache.bookkeeper.mledger.ManagedLedgerException$CursorAlreadyClosedException: pulsar.dedup cursor already closed ``` ### Anything else? _No response_ ### Are you willing to submit a PR? - [X] I'm willing to submit a PR! -- 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: commits-unsubscr...@pulsar.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [improve][admin] Improve error message when attempt to delete a partitioned topic with a wrong API [pulsar]
poorbarcode opened a new pull request, #23002: URL: https://github.com/apache/pulsar/pull/23002 ### Motivation & Modifications The correct usage of deleting topics: - `non-partitioned`: `pulsar-admin topics delete ` - `partitioned`: `pulsar-admin topics delete-partitioned-topic ` When you attempt to delete a partitioned topic with `pulsar-admin topics delete `, you will get a `404` error: `tpxxx is not found`. We would better change the error message to `tpxxx is a partitioned topic, please call delete-partitioned-topic instead`. And if use get a `404`, they will assume it has been deleted, so change the response code to `409`. ### Documentation - [ ] `doc` - [ ] `doc-required` - [x] `doc-not-needed` - [ ] `doc-complete` ### Matching PR in forked repository PR in forked repository: x -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
ivan-penchev commented on PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#issuecomment-2208768095 > @ivan-penchev I noticed this PR doesn't add a test, could you add a test for the producer and consumer with this logger? @nodece I appreciate the need for a test to accompany this update. I'm willing to write one, but I could use some assistance, given that the current test suite doesn't cover the logger. 1. Where should I place the test? 2. What should I be testing? For 2 you could see - in my PR description, I did an integration test where I started a producer with logrus, and producer with slog - and then I compared the output of both loggers, in terms of length and keys (since I was doing json). But this seems too complicated and not needed for what I was trying to test. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on code in PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1665562539 ## go.mod: ## @@ -1,6 +1,6 @@ module github.com/apache/pulsar-client-go -go 1.18 +go 1.20 Review Comment: Good catch, this should be 1.20. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#issuecomment-2208723135 @ivan-penchev I noticed this PR doesn't add a test, could you add a test for the producer and consumer with this logger? -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
ivan-penchev commented on code in PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1665552093 ## pulsar/log/wrapper_slog.go: ## @@ -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 log Review Comment: This is an elegant and incredible suggestion, I knew about build constrains, but I never had the need to use them. Doing it, and reverting the go version upgrade. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
ivan-penchev commented on code in PR #1234:
URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1665547176
##
pulsar/log/wrapper_slog.go:
##
@@ -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 log
+
+import (
+ "fmt"
+ "log/slog"
+)
+
+type slogWrapper struct {
+ logger *slog.Logger
+}
+
+func (s *slogWrapper) Debug(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Debug(message)
+}
+
+func (s *slogWrapper) Info(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Info(message)
+}
+
+func (s *slogWrapper) Error(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Error(message)
+}
+
+func (s *slogWrapper) Warn(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Warn(message)
+}
+
+func (s *slogWrapper) Debugf(format string, args ...any) {
+ s.logger.Debug(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) Infof(format string, args ...any) {
+ s.logger.Info(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) Warnf(format string, args ...any) {
+ s.logger.Warn(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) Errorf(format string, args ...any) {
+ s.logger.Error(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) SubLogger(fields Fields) Logger {
+ return {
+ logger: s.logger.With(pulsarFieldsToKVSlice(fields)...),
+ }
+}
+
+func (s *slogWrapper) WithError(err error) Entry {
+ return s.WithField("error", err)
+}
+
+func (s *slogWrapper) WithField(name string, value any) Entry {
+ return {
+ logger: s.logger.With(name, value),
+ }
+}
+
+func (s *slogWrapper) WithFields(fields Fields) Entry {
+ return {
+ logger: s.logger.With(pulsarFieldsToKVSlice(fields)...),
+ }
+}
+
+func NewLoggerWithSlog(logger *slog.Logger) Logger {
+ return {
+ logger: logger,
+ }
+}
+
+func pulsarFieldsToKVSlice(f Fields) []any {
+ ret := make([]any, 0, len(f)*2)
+ for k, v := range f {
+ ret = append(ret, k, v)
+ }
+ return ret
+}
+
+func (s *slogWrapper) tryDetermineMessage(value ...any) string {
Review Comment:
Great suggestion, maybe I tried to over engineer it a bit :D
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
(pulsar) branch branch-3.2 updated: [fix][ci] Fix OWASP Dependency Check download by using NVD API key (#22999)
This is an automated email from the ASF dual-hosted git repository.
lhotari pushed a commit to branch branch-3.2
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-3.2 by this push:
new 3a125c4169c [fix][ci] Fix OWASP Dependency Check download by using NVD
API key (#22999)
3a125c4169c is described below
commit 3a125c4169cbb55f6f05e50e3bd722609faf3bb2
Author: Lari Hotari
AuthorDate: Thu Jul 4 12:41:21 2024 +0300
[fix][ci] Fix OWASP Dependency Check download by using NVD API key (#22999)
(cherry picked from commit 8b7754f11f113af9d341a460795d0c7b8095f594)
# Conflicts:
# .github/workflows/ci-owasp-dependency-check.yaml
# pom.xml
---
.github/workflows/ci-owasp-dependency-check.yaml | 80 +---
.github/workflows/pulsar-ci.yaml | 7 +--
distribution/io/pom.xml | 1 -
pom.xml | 14 -
pulsar-io/docs/pom.xml | 1 -
pulsar-io/flume/pom.xml | 1 -
pulsar-io/hbase/pom.xml | 1 -
pulsar-io/hdfs2/pom.xml | 7 +--
pulsar-io/hdfs3/pom.xml | 9 ++-
tiered-storage/file-system/pom.xml | 1 -
10 files changed, 79 insertions(+), 43 deletions(-)
diff --git a/.github/workflows/ci-owasp-dependency-check.yaml
b/.github/workflows/ci-owasp-dependency-check.yaml
index 0ee1275bdfe..a70f4a82ff1 100644
--- a/.github/workflows/ci-owasp-dependency-check.yaml
+++ b/.github/workflows/ci-owasp-dependency-check.yaml
@@ -24,7 +24,9 @@ on:
workflow_dispatch:
env:
- MAVEN_OPTS: -Xss1500k -Xmx1024m
-Daether.connector.http.reuseConnections=false
-Daether.connector.requestTimeout=6 -Dhttp.keepAlive=false
-Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard
-Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.http.retryHandler.requestSentEnabled=true
-Dmaven.wagon.http.serviceUnavailableRetryStrategy.class=standard
-Dmaven.wagon.rto=6
+ MAVEN_OPTS: -Xss1500k -Xmx1500m
-Daether.connector.http.reuseConnections=false
-Daether.connector.requestTimeout=6 -Dhttp.keepAlive=false
-Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard
-Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.http.retryHandler.requestSentEnabled=true
-Dmaven.wagon.http.serviceUnavailableRetryStrategy.class=standard
-Dmaven.wagon.rto=6
+ JDK_DISTRIBUTION: corretto
+ NIST_NVD_API_KEY: ${{ secrets.NIST_NVD_API_KEY }}
jobs:
run-owasp-dependency-check:
@@ -34,62 +36,96 @@ jobs:
JOB_NAME: Check ${{ matrix.branch }}
GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GE_ACCESS_TOKEN }}
runs-on: ubuntu-22.04
-timeout-minutes: 45
+timeout-minutes: 75
strategy:
fail-fast: false
+ max-parallel: 1
matrix:
include:
- branch: master
- - branch: branch-3.1
+ - branch: branch-3.3
+ - branch: branch-3.2
- branch: branch-3.0
- - branch: branch-2.11
- - branch: branch-2.10
-jdk: 11
- - branch: branch-2.9
-jdk: 11
- - branch: branch-2.8
-jdk: 11
steps:
- name: checkout
-uses: actions/checkout@v3
+uses: actions/checkout@v4
with:
ref: ${{ matrix.branch }}
- name: Tune Runner VM
uses: ./.github/actions/tune-runner-vm
- - name: Cache local Maven repository
-uses: actions/cache@v3
+ - name: Restore Maven repository cache
+uses: actions/cache/restore@v4
timeout-minutes: 5
with:
path: |
~/.m2/repository/*/*/*
!~/.m2/repository/org/apache/pulsar
- key: ${{ runner.os }}-m2-dependencies-owasp-${{
hashFiles('**/pom.xml') }}
+ key: ${{ runner.os }}-m2-dependencies-all-${{
hashFiles('**/pom.xml') }}
restore-keys: |
-${{ runner.os }}-m2-dependencies-all-${{ hashFiles('**/pom.xml') }}
${{ runner.os }}-m2-dependencies-core-modules-${{
hashFiles('**/pom.xml') }}
${{ runner.os }}-m2-dependencies-core-modules-
- name: Set up JDK ${{ matrix.jdk || '17' }}
-uses: actions/setup-java@v3
+uses: actions/setup-java@v4
with:
- distribution: 'temurin'
+ distribution: ${{ env.JDK_DISTRIBUTION }}
java-version: ${{ matrix.jdk || '17' }}
- name: run install by skip tests
-run: mvn -B -ntp clean install -DskipTests -Dspotbugs.skip=true
-Dlicense.skip=true -Dcheckstyle.skip=true -Drat.skip=true -DskipDocker=true
+run: mvn -B -ntp clean install -DskipTests -Dspotbugs.skip=true
-Dlicense.skip=true -Dcheckstyle.skip=true -Drat.skip=true -DskipDocker=true
-DnarPluginPhase=none -pl
(pulsar) branch branch-3.3 updated: [fix][ci] Fix OWASP Dependency Check download by using NVD API key (#22999)
This is an automated email from the ASF dual-hosted git repository.
lhotari pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-3.3 by this push:
new 589f2831aae [fix][ci] Fix OWASP Dependency Check download by using NVD
API key (#22999)
589f2831aae is described below
commit 589f2831aae3323ad3c80b30bcb1d899e6080439
Author: Lari Hotari
AuthorDate: Thu Jul 4 12:41:21 2024 +0300
[fix][ci] Fix OWASP Dependency Check download by using NVD API key (#22999)
(cherry picked from commit 8b7754f11f113af9d341a460795d0c7b8095f594)
# Conflicts:
# pom.xml
---
.github/workflows/ci-owasp-dependency-check.yaml | 20
.github/workflows/pulsar-ci.yaml | 9 -
distribution/io/pom.xml | 1 -
pom.xml | 14 +++---
pulsar-io/docs/pom.xml | 1 -
pulsar-io/flume/pom.xml | 1 -
pulsar-io/hbase/pom.xml | 1 -
pulsar-io/hdfs2/pom.xml | 7 +++
pulsar-io/hdfs3/pom.xml | 9 -
tiered-storage/file-system/pom.xml | 1 -
10 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/.github/workflows/ci-owasp-dependency-check.yaml
b/.github/workflows/ci-owasp-dependency-check.yaml
index a273e902c88..a70f4a82ff1 100644
--- a/.github/workflows/ci-owasp-dependency-check.yaml
+++ b/.github/workflows/ci-owasp-dependency-check.yaml
@@ -24,8 +24,9 @@ on:
workflow_dispatch:
env:
- MAVEN_OPTS: -Xss1500k -Xmx1024m
-Daether.connector.http.reuseConnections=false
-Daether.connector.requestTimeout=6 -Dhttp.keepAlive=false
-Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard
-Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.http.retryHandler.requestSentEnabled=true
-Dmaven.wagon.http.serviceUnavailableRetryStrategy.class=standard
-Dmaven.wagon.rto=6
+ MAVEN_OPTS: -Xss1500k -Xmx1500m
-Daether.connector.http.reuseConnections=false
-Daether.connector.requestTimeout=6 -Dhttp.keepAlive=false
-Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard
-Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.http.retryHandler.requestSentEnabled=true
-Dmaven.wagon.http.serviceUnavailableRetryStrategy.class=standard
-Dmaven.wagon.rto=6
JDK_DISTRIBUTION: corretto
+ NIST_NVD_API_KEY: ${{ secrets.NIST_NVD_API_KEY }}
jobs:
run-owasp-dependency-check:
@@ -42,12 +43,9 @@ jobs:
matrix:
include:
- branch: master
+ - branch: branch-3.3
- branch: branch-3.2
- - branch: branch-3.1
- branch: branch-3.0
- - branch: branch-2.11
- - branch: branch-2.10
-jdk: 11
steps:
- name: checkout
@@ -58,16 +56,14 @@ jobs:
- name: Tune Runner VM
uses: ./.github/actions/tune-runner-vm
- - name: Cache local Maven repository
-uses: actions/cache@v4
+ - name: Restore Maven repository cache
+uses: actions/cache/restore@v4
timeout-minutes: 5
with:
path: |
~/.m2/repository/*/*/*
!~/.m2/repository/org/apache/pulsar
-!~/.m2/repository/org/owasp/dependency-check-data
key: ${{ runner.os }}-m2-dependencies-all-${{
hashFiles('**/pom.xml') }}
- lookup-only: true
restore-keys: |
${{ runner.os }}-m2-dependencies-core-modules-${{
hashFiles('**/pom.xml') }}
${{ runner.os }}-m2-dependencies-core-modules-
@@ -79,7 +75,7 @@ jobs:
java-version: ${{ matrix.jdk || '17' }}
- name: run install by skip tests
-run: mvn -B -ntp clean install -DskipTests -Dspotbugs.skip=true
-Dlicense.skip=true -Dcheckstyle.skip=true -Drat.skip=true -DskipDocker=true
+run: mvn -B -ntp clean install -DskipTests -Dspotbugs.skip=true
-Dlicense.skip=true -Dcheckstyle.skip=true -Drat.skip=true -DskipDocker=true
-DnarPluginPhase=none -pl '!distribution/io,!distribution/offloaders'
- name: OWASP cache key weeknum
id: get-weeknum
@@ -89,7 +85,7 @@ jobs:
- name: Restore OWASP Dependency Check data
id: restore-owasp-dependency-check-data
-uses: actions/cache/restore@v3
+uses: actions/cache/restore@v4
timeout-minutes: 5
with:
path: ~/.m2/repository/org/owasp/dependency-check-data
@@ -105,7 +101,7 @@ jobs:
- name: Save OWASP Dependency Check data
if: ${{ steps.update-owasp-dependency-check-data.outcome == 'success'
}}
-uses: actions/cache/save@v3
+uses: actions/cache/save@v4
timeout-minutes: 5
with:
path: ~/.m2/repository/org/owasp/dependency-check-data
diff --git
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on code in PR #1234:
URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1665495312
##
pulsar/log/wrapper_slog.go:
##
@@ -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 log
+
+import (
+ "fmt"
+ "log/slog"
+)
+
+type slogWrapper struct {
+ logger *slog.Logger
+}
+
+func (s *slogWrapper) Debug(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Debug(message)
+}
+
+func (s *slogWrapper) Info(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Info(message)
+}
+
+func (s *slogWrapper) Error(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Error(message)
+}
+
+func (s *slogWrapper) Warn(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Warn(message)
+}
+
+func (s *slogWrapper) Debugf(format string, args ...any) {
+ s.logger.Debug(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) Infof(format string, args ...any) {
+ s.logger.Info(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) Warnf(format string, args ...any) {
+ s.logger.Warn(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) Errorf(format string, args ...any) {
+ s.logger.Error(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) SubLogger(fields Fields) Logger {
+ return {
+ logger: s.logger.With(pulsarFieldsToKVSlice(fields)...),
+ }
+}
+
+func (s *slogWrapper) WithError(err error) Entry {
+ return s.WithField("error", err)
+}
+
+func (s *slogWrapper) WithField(name string, value any) Entry {
+ return {
+ logger: s.logger.With(name, value),
+ }
+}
+
+func (s *slogWrapper) WithFields(fields Fields) Entry {
+ return {
+ logger: s.logger.With(pulsarFieldsToKVSlice(fields)...),
+ }
+}
+
+func NewLoggerWithSlog(logger *slog.Logger) Logger {
+ return {
+ logger: logger,
+ }
+}
+
+func pulsarFieldsToKVSlice(f Fields) []any {
+ ret := make([]any, 0, len(f)*2)
+ for k, v := range f {
+ ret = append(ret, k, v)
+ }
+ return ret
+}
+
+func (s *slogWrapper) tryDetermineMessage(value ...any) string {
Review Comment:
Usage:
```
logger.Info("message id: ", "1:1")
```
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on code in PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1665491465 ## README.md: ## @@ -38,7 +38,7 @@ CGo-based library. ## Requirements -- Go 1.20+ +- Go 1.21+ Review Comment: Revert. ## README.md: ## @@ -152,7 +152,7 @@ Run the tests: Run the tests with specific versions of GOLANG and PULSAR: -make test GOLANG_VERSION=1.20 PULSAR_VERSION=2.10.0 +make test GOLANG_VERSION=1.21.0 PULSAR_VERSION=2.10.0 Review Comment: Revert. ## go.mod: ## @@ -1,6 +1,6 @@ module github.com/apache/pulsar-client-go -go 1.18 +go 1.21 Review Comment: Revert. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on code in PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1665491316 ## README.md: ## @@ -152,7 +152,7 @@ Run the tests: Run the tests with specific versions of GOLANG and PULSAR: -make test GOLANG_VERSION=1.20 PULSAR_VERSION=2.10.0 +make test GOLANG_VERSION=1.21.0 PULSAR_VERSION=2.10.0 Review Comment: Revert. ## README.md: ## @@ -38,7 +38,7 @@ CGo-based library. ## Requirements -- Go 1.20+ +- Go 1.21+ Review Comment: Revert. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on code in PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1665490761 ## pulsar/log/wrapper_slog.go: ## @@ -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 log Review Comment: ```suggestion //go:build go1.21 package log ``` I suggest you use Go build constraints. When you use go1.21+, this file will be compiled, otherwise, not. This can keep us compatible with the go 1.20, and you need to revert the version changes. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on code in PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1665490761 ## pulsar/log/wrapper_slog.go: ## @@ -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 log Review Comment: ```suggestion //go:build go1.21 package log ``` I suggest you use Go build constraints. When you use go1.21+, this file will be compiled, otherwise, not. This can keep us compatible with the go 1.20. ## go.mod: ## @@ -1,6 +1,6 @@ module github.com/apache/pulsar-client-go -go 1.18 +go 1.21 Review Comment: Revert. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
nodece commented on code in PR #1234:
URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1665485246
##
pulsar/log/wrapper_slog.go:
##
@@ -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 log
+
+import (
+ "fmt"
+ "log/slog"
+)
+
+type slogWrapper struct {
+ logger *slog.Logger
+}
+
+func (s *slogWrapper) Debug(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Debug(message)
+}
+
+func (s *slogWrapper) Info(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Info(message)
+}
+
+func (s *slogWrapper) Error(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Error(message)
+}
+
+func (s *slogWrapper) Warn(args ...any) {
+ message := s.tryDetermineMessage(args)
+ s.logger.Warn(message)
+}
+
+func (s *slogWrapper) Debugf(format string, args ...any) {
+ s.logger.Debug(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) Infof(format string, args ...any) {
+ s.logger.Info(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) Warnf(format string, args ...any) {
+ s.logger.Warn(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) Errorf(format string, args ...any) {
+ s.logger.Error(fmt.Sprintf(format, args...))
+}
+
+func (s *slogWrapper) SubLogger(fields Fields) Logger {
+ return {
+ logger: s.logger.With(pulsarFieldsToKVSlice(fields)...),
+ }
+}
+
+func (s *slogWrapper) WithError(err error) Entry {
+ return s.WithField("error", err)
+}
+
+func (s *slogWrapper) WithField(name string, value any) Entry {
+ return {
+ logger: s.logger.With(name, value),
+ }
+}
+
+func (s *slogWrapper) WithFields(fields Fields) Entry {
+ return {
+ logger: s.logger.With(pulsarFieldsToKVSlice(fields)...),
+ }
+}
+
+func NewLoggerWithSlog(logger *slog.Logger) Logger {
+ return {
+ logger: logger,
+ }
+}
+
+func pulsarFieldsToKVSlice(f Fields) []any {
+ ret := make([]any, 0, len(f)*2)
+ for k, v := range f {
+ ret = append(ret, k, v)
+ }
+ return ret
+}
+
+func (s *slogWrapper) tryDetermineMessage(value ...any) string {
Review Comment:
```suggestion
func (s *slogWrapper) tryDetermineMessage(value ...any) string {
return fmt.Sprint(args...)
}
```
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][ci] Fix OWASP Dependency Check download by using NVD API key [pulsar]
lhotari merged PR #22999: URL: https://github.com/apache/pulsar/pull/22999 -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(pulsar) branch master updated: [fix][ci] Fix OWASP Dependency Check download by using NVD API key (#22999)
This is an automated email from the ASF dual-hosted git repository.
lhotari pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push:
new 8b7754f11f1 [fix][ci] Fix OWASP Dependency Check download by using NVD
API key (#22999)
8b7754f11f1 is described below
commit 8b7754f11f113af9d341a460795d0c7b8095f594
Author: Lari Hotari
AuthorDate: Thu Jul 4 12:41:21 2024 +0300
[fix][ci] Fix OWASP Dependency Check download by using NVD API key (#22999)
---
.github/workflows/ci-owasp-dependency-check.yaml | 20
.github/workflows/pulsar-ci.yaml | 9 -
distribution/io/pom.xml | 1 -
pom.xml | 14 +++---
pulsar-io/docs/pom.xml | 1 -
pulsar-io/flume/pom.xml | 1 -
pulsar-io/hbase/pom.xml | 1 -
pulsar-io/hdfs2/pom.xml | 7 +++
pulsar-io/hdfs3/pom.xml | 9 -
tiered-storage/file-system/pom.xml | 1 -
10 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/.github/workflows/ci-owasp-dependency-check.yaml
b/.github/workflows/ci-owasp-dependency-check.yaml
index a273e902c88..a70f4a82ff1 100644
--- a/.github/workflows/ci-owasp-dependency-check.yaml
+++ b/.github/workflows/ci-owasp-dependency-check.yaml
@@ -24,8 +24,9 @@ on:
workflow_dispatch:
env:
- MAVEN_OPTS: -Xss1500k -Xmx1024m
-Daether.connector.http.reuseConnections=false
-Daether.connector.requestTimeout=6 -Dhttp.keepAlive=false
-Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard
-Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.http.retryHandler.requestSentEnabled=true
-Dmaven.wagon.http.serviceUnavailableRetryStrategy.class=standard
-Dmaven.wagon.rto=6
+ MAVEN_OPTS: -Xss1500k -Xmx1500m
-Daether.connector.http.reuseConnections=false
-Daether.connector.requestTimeout=6 -Dhttp.keepAlive=false
-Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard
-Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.http.retryHandler.requestSentEnabled=true
-Dmaven.wagon.http.serviceUnavailableRetryStrategy.class=standard
-Dmaven.wagon.rto=6
JDK_DISTRIBUTION: corretto
+ NIST_NVD_API_KEY: ${{ secrets.NIST_NVD_API_KEY }}
jobs:
run-owasp-dependency-check:
@@ -42,12 +43,9 @@ jobs:
matrix:
include:
- branch: master
+ - branch: branch-3.3
- branch: branch-3.2
- - branch: branch-3.1
- branch: branch-3.0
- - branch: branch-2.11
- - branch: branch-2.10
-jdk: 11
steps:
- name: checkout
@@ -58,16 +56,14 @@ jobs:
- name: Tune Runner VM
uses: ./.github/actions/tune-runner-vm
- - name: Cache local Maven repository
-uses: actions/cache@v4
+ - name: Restore Maven repository cache
+uses: actions/cache/restore@v4
timeout-minutes: 5
with:
path: |
~/.m2/repository/*/*/*
!~/.m2/repository/org/apache/pulsar
-!~/.m2/repository/org/owasp/dependency-check-data
key: ${{ runner.os }}-m2-dependencies-all-${{
hashFiles('**/pom.xml') }}
- lookup-only: true
restore-keys: |
${{ runner.os }}-m2-dependencies-core-modules-${{
hashFiles('**/pom.xml') }}
${{ runner.os }}-m2-dependencies-core-modules-
@@ -79,7 +75,7 @@ jobs:
java-version: ${{ matrix.jdk || '17' }}
- name: run install by skip tests
-run: mvn -B -ntp clean install -DskipTests -Dspotbugs.skip=true
-Dlicense.skip=true -Dcheckstyle.skip=true -Drat.skip=true -DskipDocker=true
+run: mvn -B -ntp clean install -DskipTests -Dspotbugs.skip=true
-Dlicense.skip=true -Dcheckstyle.skip=true -Drat.skip=true -DskipDocker=true
-DnarPluginPhase=none -pl '!distribution/io,!distribution/offloaders'
- name: OWASP cache key weeknum
id: get-weeknum
@@ -89,7 +85,7 @@ jobs:
- name: Restore OWASP Dependency Check data
id: restore-owasp-dependency-check-data
-uses: actions/cache/restore@v3
+uses: actions/cache/restore@v4
timeout-minutes: 5
with:
path: ~/.m2/repository/org/owasp/dependency-check-data
@@ -105,7 +101,7 @@ jobs:
- name: Save OWASP Dependency Check data
if: ${{ steps.update-owasp-dependency-check-data.outcome == 'success'
}}
-uses: actions/cache/save@v3
+uses: actions/cache/save@v4
timeout-minutes: 5
with:
path: ~/.m2/repository/org/owasp/dependency-check-data
diff --git a/.github/workflows/pulsar-ci.yaml b/.github/workflows/pulsar-ci.yaml
index 8decde1c999..828f876f131 100644
--- a/.github/workflows/pulsar-ci.yaml
Re: [PR] [fix][client] Fix pattern consumer create crash if a part of partitions of a topic have been deleted [pulsar]
codecov-commenter commented on PR #22854: URL: https://github.com/apache/pulsar/pull/22854#issuecomment-2208505903 ## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/22854?dropdown=coverage=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) Report Attention: Patch coverage is `80.47945%` with `57 lines` in your changes missing coverage. Please review. > Project coverage is 73.45%. Comparing base [(`bbc6224`)](https://app.codecov.io/gh/apache/pulsar/commit/bbc62245c5ddba1de4b1e7cee4ab49334bc36277?dropdown=coverage=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) to head [(`af01c4f`)](https://app.codecov.io/gh/apache/pulsar/commit/af01c4f27af19a95f75c60360ea78e961c487eae?dropdown=coverage=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). > Report is 435 commits behind head on master. Additional details and impacted files [](https://app.codecov.io/gh/apache/pulsar/pull/22854?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) ```diff @@ Coverage Diff @@ ## master #22854 +/- ## - Coverage 73.57% 73.45% -0.12% - Complexity3262433324 +700 Files 1877 1911 +34 Lines139502 143274+3772 Branches 1529915629 +330 + Hits 102638 105247+2609 - Misses2890829980+1072 - Partials 7956 8047 +91 ``` | [Flag](https://app.codecov.io/gh/apache/pulsar/pull/22854/flags?src=pr=flags_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | Coverage Δ | | |---|---|---| | [inttests](https://app.codecov.io/gh/apache/pulsar/pull/22854/flags?src=pr=flag_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | `27.53% <7.19%> (+2.95%)` | :arrow_up: | | [systests](https://app.codecov.io/gh/apache/pulsar/pull/22854/flags?src=pr=flag_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | `24.75% <17.12%> (+0.42%)` | :arrow_up: | | [unittests](https://app.codecov.io/gh/apache/pulsar/pull/22854/flags?src=pr=flag_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | `72.49% <80.47%> (-0.35%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/apache/pulsar/pull/22854?dropdown=coverage=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | Coverage Δ | | |---|---|---| | [...va/org/apache/pulsar/client/impl/ConsumerBase.java](https://app.codecov.io/gh/apache/pulsar/pull/22854?src=pr=tree=pulsar-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fimpl%2FConsumerBase.java_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL0NvbnN1bWVyQmFzZS5qYXZh) | `74.39% <100.00%> (+0.26%)` | :arrow_up: | | [...rg/apache/pulsar/client/impl/TopicListWatcher.java](https://app.codecov.io/gh/apache/pulsar/pull/22854?src=pr=tree=pulsar-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fimpl%2FTopicListWatcher.java_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1RvcGljTGlzdFdhdGNoZXIuamF2YQ==) | `66.91% <100.00%> (-0.95%)` | :arrow_down: | | [...g/apache/pulsar/common/lookup/GetTopicsResult.java](https://app.codecov.io/gh/apache/pulsar/pull/22854?src=pr=tree=pulsar-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fcommon%2Flookup%2FGetTopicsResult.java_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi9sb29rdXAvR2V0VG9waWNzUmVzdWx0LmphdmE=) | `92.50% <100.00%> (+30.00%)` | :arrow_up: | |
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
crossoverJie commented on PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#issuecomment-2208412685 > Edit: @crossoverJie I decided to give the upgrade a chance, after reading go's release policy. I hope I haven't done anything bad. I have no objections to upgrading to 1.21. We've already skipped 1.18 to 1.19, so skipping 1.20 as well doesn't seem problematic. cc @nodece @RobertIndie -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
ivan-penchev commented on PR #1234: URL: https://github.com/apache/pulsar-client-go/pull/1234#issuecomment-2208336183 > @ivan-penchev In this [PR](https://github.com/apache/pulsar-client-go/pull/1230), the Go version is upgraded to a minimum of 1.20, but the slog package is officially used in [1.21](https://go.dev/blog/slog). > > This change requires upgrading the Go version to 1.21+. Upgrading the Go version looks to be a clear and well defined step. Is it practical to consider a minimum version upgrade for this PR? Considering Go is considered feature-complete and lacks an LTS version, it's generally expected to maintain compatibility with the last two minor versions, as seen on their download page (https://go.dev/dl/). But I've read some issues here indicating a more conservative update strategy for this SDK. Is that the case? -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [feat] added a slog wrapper of the logger interface [pulsar-client-go]
ivan-penchev commented on code in PR #1234:
URL: https://github.com/apache/pulsar-client-go/pull/1234#discussion_r1665275431
##
pulsar/log/wrapper_slog.go:
##
@@ -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 log
+
+import (
+ "fmt"
+ "log/slog"
+)
+
+type slogWrapper struct {
+ logger *slog.Logger
+}
+
+func (s *slogWrapper) Debug(args ...interface{}) {
Review Comment:
Sure any and interface{} are basically type aliases, and the only difference
I have seen is when doing generics type inference, which we do not have here :)
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
(pulsar-client-cpp) branch main updated: [CI] Use macos-12 to build macOS libraries (#433)
This is an automated email from the ASF dual-hosted git repository. mmerli pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/pulsar-client-cpp.git The following commit(s) were added to refs/heads/main by this push: new 35bf161 [CI] Use macos-12 to build macOS libraries (#433) 35bf161 is described below commit 35bf161ba25c9ea073b730e3dcdaa50c30703bcb Author: Yunze Xu AuthorDate: Thu Jul 4 15:28:03 2024 +0800 [CI] Use macos-12 to build macOS libraries (#433) --- .github/workflows/ci-build-binary-artifacts.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-build-binary-artifacts.yaml b/.github/workflows/ci-build-binary-artifacts.yaml index 63644e5..7984410 100644 --- a/.github/workflows/ci-build-binary-artifacts.yaml +++ b/.github/workflows/ci-build-binary-artifacts.yaml @@ -197,7 +197,7 @@ jobs: package-macos: name: Build macOS libraries -runs-on: macos-latest +runs-on: macos-12 timeout-minutes: 500 strategy:
Re: [PR] [CI] Use macos-12 to build macOS libraries [pulsar-client-cpp]
merlimat merged PR #433: URL: https://github.com/apache/pulsar-client-cpp/pull/433 -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(pulsar) branch master updated: [improve][pip] PIP-337: SSL Factory Plugin to customize SSL Context and SSL Engine generation (#22016)
This is an automated email from the ASF dual-hosted git repository. mmerli pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git The following commit(s) were added to refs/heads/master by this push: new 2086cc46c88 [improve][pip] PIP-337: SSL Factory Plugin to customize SSL Context and SSL Engine generation (#22016) 2086cc46c88 is described below commit 2086cc46c882df7fb2855a3cdb2580e1bc3adc5b Author: Apurva007 AuthorDate: Thu Jul 4 00:22:19 2024 -0700 [improve][pip] PIP-337: SSL Factory Plugin to customize SSL Context and SSL Engine generation (#22016) Co-authored-by: Apurva Telang --- pip/pip-337.md | 382 + 1 file changed, 382 insertions(+) diff --git a/pip/pip-337.md b/pip/pip-337.md new file mode 100644 index 000..283bb9710de --- /dev/null +++ b/pip/pip-337.md @@ -0,0 +1,382 @@ +# PIP-337: SSL Factory Plugin to customize SSLContext/SSLEngine generation + +# Background knowledge +Apache Pulsar supports TLS encrypted communication between the clients and servers. The TLS encryption setup requires +loading the TLS certificates and its respective passwords to generate the SSL Context. Pulsar supports loading these +certificates and passwords via the filesystem. It supports both Java based Keystores/Truststores and TLS information in +".crt", ".pem" & ".key" formats. This information is refreshed based on a configurable interval. + +Apache Pulsar internally uses 3 different frameworks for connection management: + +- Netty: Connection management for Pulsar server and client that understands Pulsar binary protocol. +- Jetty: HTTP Server creation for Pulsar Admin and websocket. Jetty Client is used by proxy for admin client calls. +- AsyncHttpClient: HTTP Client creation for Admin client and HTTP Lookup + +Each of the above frameworks supports customizing the generation of the SSL Context and SSL Engine. Currently, Pulsar +uses these features to feed the SSL Context via its internal security tools after loading the file based certificates. +One of the issues of using these features is that pulsar tries to bootstrap the SSL Context in multiple ways to suit +each framework and file type. + +```mermaid +flowchart TB +Proxy.DirectProxyHandler --> NettyClientSslContextRefresher +Proxy.DirectProxyHandler --> NettySSLContextAutoRefreshBuilder +Proxy.AdminProxyHandler --> KeyStoreSSLContext +Proxy.AdminProxyHandler --> SecurityUtility +Proxy.ServiceChannelInitializer --> NettySSLContextAutoRefreshBuilder +Proxy.ServiceChannelInitializer --> NettyServerSslContextBuilder +Broker.PulsarChannelInitializer --> NettyServerSslContextBuilder +Broker.PulsarChannelInitializer --> NettySSLContextAutoRefreshBuilder +Client.PulsarChannelInitializer --> NettySSLContextAutoRefreshBuilder +Client.PulsarChannelInitializer --> SecurityUtility +Broker.WebService --> JettySSlContextFactory +Proxy.WebServer --> JettySSlContextFactory +PulsarAdmin --> AsyncHttpConnector +AsyncHttpConnector --> KeyStoreSSLContext +AsyncHttpConnector --> SecurityUtility +JettySSlContextFactory --> NetSslContextBuilder +JettySSlContextFactory --> DefaultSslContextBuilder +NettyClientSslContextRefresher -.-> SslContextAutoRefreshBuilder +NettySSLContextAutoRefreshBuilder -.-> SslContextAutoRefreshBuilder +NettyServerSslContextBuilder -.-> SslContextAutoRefreshBuilder +NetSslContextBuilder -.-> SslContextAutoRefreshBuilder +DefaultSslContextBuilder -.-> SslContextAutoRefreshBuilder +Client.HttpLookup.HttpClient --> KeyStoreSSLContext +Client.HttpLookup.HttpClient --> SecurityUtility +SecurityUtility -.-> KeyManagerProxy +SecurityUtility -.-> TrustManagerProxy +``` +The above diagram is an example of the complexity of the TLS encryption setup within Pulsar. The above diagram only +contains the basic components of Pulsar excluding Websockets, Functions, etc. + +Pulsar uses 2 base classes to load the TLS information. + +- `SecurityUtility`: It loads files of type ".crt", ".pem" and ".key" and converts it into SSL Context. This SSL Context +can be of type `io.netty.handler.ssl.SslContext` or `javax.net.ssl.SSLContext` based on the caller. Security Utility +can be used to create SSL Context that internally has KeyManager and Trustmanager proxies that load cert changes +dynamically. +- `KeyStoreSSLContext`: It loads files of type Java Keystore/Truststore and converts it into SSL Context. This SSL +Context will be of type `javax.net.ssl.SSLContext`. This is always used to create the SSL Engine. + +Each of the above classes are either directly used by Pulsar Clients or used via implementations of the abstract class +`SslContextAutoRefreshBuilder`. + +- `SslContextAutoRefreshBuilder` - This abstract class is used to refresh certificates at a configurable interval. It +internally provides a public API to return
Re: [PR] [improve][pip] PIP-337: SSL Factory Plugin to customize SSL Context and SSL Engine generation [pulsar]
merlimat merged PR #22016: URL: https://github.com/apache/pulsar/pull/22016 -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
