Skip to content

Conversation

@skeeey
Copy link
Member

@skeeey skeeey commented Sep 17, 2025

Summary

Related issue(s)

Fixes #

Summary by CodeRabbit

  • New Features

    • Enhanced MQTT observability: added integrated debug and error logging for CloudEvents over MQTT, improving insight into connection and runtime behavior. No breaking changes.
  • Chores

    • Updated dependencies to newer versions (CloudEvents SDKs, Paho MQTT, testing, and Go x libraries) for improved stability, compatibility, and performance.

@openshift-ci openshift-ci bot requested review from deads2k and qiujian16 September 17, 2025 02:34
@coderabbitai
Copy link

coderabbitai bot commented Sep 17, 2025

Walkthrough

Introduces Paho MQTT logging integration by adding debug and error logger types and wiring them into MQTT CloudEvents protocol options. Updates go.mod to newer versions of cloudevents SDKs, Paho, testify, and several golang.org/x dependencies. No public API signatures changed.

Changes

Cohort / File(s) Summary
MQTT logging integration
pkg/cloudevents/generic/options/mqtt/options.go, pkg/cloudevents/generic/options/mqtt/logger.go
Added PahoDebugLogger and PahoErrorLogger implementing log interfaces; updated GetCloudEventsProtocol to include WithDebugLogger and WithErrorLogger alongside existing connect option.
Module dependency updates
go.mod
Bumped versions for cloudevents SDKs, eclipse/paho.golang, testify, and multiple indirect golang.org/x packages.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • qiujian16
  • deads2k

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description contains only the repository PR template with empty "Summary" and "Related issue(s)" sections (the "Fixes #" line has no issue number) and provides no implementation details, motivation, testing instructions, or impact notes; the pr_objectives confirm the description is unfilled and notes a dependency and hold, but those details are not recorded in the description. Because the required sections are missing or blank, the description is incomplete and does not meet the repository's template requirements. Populate the "Summary" with a concise description of what changed, why, and the expected impact; add Related issue(s) with proper issue numbers or remove the "Fixes #" placeholder; document testing steps, any migration or compatibility notes, and the dependency on the referenced cloudevents/sdk-go PR; remove the hold/do-not-merge label when the description and tests are complete.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title ":seedling: enable mqtt logger info" is concise, uses the repository's emoji prefix convention, and aligns with the primary change in pkg/cloudevents/generic/options/mqtt which adds PahoDebugLogger and PahoErrorLogger and wires WithDebugLogger/WithErrorLogger into the protocol options. It communicates the intent to enable MQTT logging, though it is slightly imprecise about the distinction between debug and error logging.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@skeeey
Copy link
Member Author

skeeey commented Sep 17, 2025

/assign @qiujian16

@skeeey
Copy link
Member Author

skeeey commented Sep 17, 2025

/hold

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
pkg/cloudevents/generic/options/mqtt/options.go (2)

228-231: Gate Paho pinger debug behind klog verbosity.

Avoid wiring a debug logger when V(4) isn’t enabled to reduce per‑ping overhead in prod.

Apply this diff:

 // setting a pinger with debug enabled
 pinger := paho.NewDefaultPinger()
-pinger.SetDebug(&PahoDebugLogger{})
+if klog.V(4).Enabled() {
+	pinger.SetDebug(&PahoDebugLogger{})
+}

Add missing import:

 import (
   "context"
   "crypto/tls"
   "fmt"
   "net"
   "os"
   "regexp"
   "strings"
   "time"
+  "k8s.io/klog/v2"
   ...
 )

236-236: Consider setting PingHandler only when KeepAlive > 0.

Small guard prevents unnecessary setup when a config sets KeepAlive=0 (disables keepalives).

Example:

config := &paho.ClientConfig{ /* ... */ }
if o.KeepAlive > 0 {
  config.PingHandler = pinger
}
pkg/cloudevents/generic/options/mqtt/log.go (1)

8-19: Minor polish: doc comment, idiomatic assert, optional early level check.

  • Add a GoDoc line for the exported type.
  • Use the conventional compile‑time assert form.
  • Optional: fast‑path return when V(4) is disabled.
-type PahoDebugLogger struct {
-}
-
-var _ log.Logger = &PahoDebugLogger{}
+// PahoDebugLogger routes Paho debug logs to klog at V(4).
+type PahoDebugLogger struct{}
+
+var _ log.Logger = (*PahoDebugLogger)(nil)
 
 func (l *PahoDebugLogger) Println(v ...interface{}) {
-	klog.V(4).Info(v...)
+	if !klog.V(4).Enabled() {
+		return
+	}
+	klog.Info(v...)
 }
 
 func (l *PahoDebugLogger) Printf(format string, v ...interface{}) {
-	klog.V(4).Infof(format, v...)
+	if !klog.V(4).Enabled() {
+		return
+	}
+	klog.Infof(format, v...)
 }

Note: Info/Infof already check verbosity; the early return is purely micro‑opt.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ef88575 and d51d069.

📒 Files selected for processing (2)
  • pkg/cloudevents/generic/options/mqtt/log.go (1 hunks)
  • pkg/cloudevents/generic/options/mqtt/options.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/cloudevents/generic/options/mqtt/options.go (1)
pkg/cloudevents/generic/options/mqtt/log.go (1)
  • PahoDebugLogger (8-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: unit
  • GitHub Check: integration
  • GitHub Check: verify
🔇 Additional comments (3)
pkg/cloudevents/generic/options/mqtt/options.go (1)

228-231: Verify DefaultPinger lifecycle across reconnects.

Known issues show DefaultPinger instances can’t be restarted and may block reconnection if reused (“Run() already called”). Please confirm mqtt_paho/v2 doesn’t attempt to reuse the same PingHandler after a disconnect, or switch to creating a fresh pinger per (re)connect cycle. (github.com)

If reuse occurs, prefer the library default pinger (omit PingHandler) or inject a new DefaultPinger on each reconnect hook (if available).

Also applies to: 236-236

pkg/cloudevents/generic/options/mqtt/log.go (2)

13-19: LGTM: logger correctly satisfies paho/log.Logger and routes to klog V(4).

Implementation is minimal and safe.


13-19: Confirm that Paho’s Logger interface hasn’t added methods in your pinned version.

Interface today includes only Println/Printf. Double‑check against your version to avoid silent breaks when upgrading. (pkg.go.dev)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
pkg/cloudevents/generic/options/mqtt/log.go (2)

22-27: Avoid duplicate logging and unify style with structured klog.

Remove stdout writes and use InfoS for consistency and better log parsing.

Apply:

 func (l *PahoDebugLogger) Printf(format string, v ...interface{}) {
-	fmt.Println("-------------")
-	fmt.Printf(format, v...)
-	fmt.Println("-------------")
-	klog.V(4).Infof(format, v...)
+	klog.V(4).InfoS("paho mqtt debug", "msg", fmt.Sprintf(format, v...))
 }

10-11: Add GoDoc for exported type.

Document the exported logger to satisfy linters and improve clarity.

Apply:

+// PahoDebugLogger forwards Eclipse Paho MQTT debug logs to klog at V=4.
 type PahoDebugLogger struct {
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d51d069 and 1efea8e.

📒 Files selected for processing (2)
  • pkg/cloudevents/generic/options/mqtt/log.go (1 hunks)
  • pkg/cloudevents/generic/options/mqtt/options.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/cloudevents/generic/options/mqtt/options.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: integration
  • GitHub Check: unit
  • GitHub Check: build
  • GitHub Check: verify
🔇 Additional comments (1)
pkg/cloudevents/generic/options/mqtt/log.go (1)

13-13: Interface conformance assertion looks good.

Compile‑time check against paho/log.Logger is correct.

@@ -0,0 +1,27 @@
package mqtt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Add repository boilerplate/SPDX header.

OCM repos typically enforce a license header check. Add the project’s standard header (or SPDX tag) to avoid CI failures.

Run to detect the expected style and where it’s missing:


🏁 Script executed:

#!/bin/bash
# Detect common license header styles in existing .go files
rg -nP 'SPDX-License-Identifier|Copyright.*Open Cluster|Apache License' -g '**/*.go' -C1
# Show files missing SPDX header
fd -e go | xargs -n1 awk 'NR==1{if($0 !~ /SPDX-License-Identifier|Copyright|Apache License/) print FILENAME}'

Length of output: 35510


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Searching for license-like patterns in .go files..."
rg -nP 'SPDX-License-Identifier|SPDX-FileCopyrightText|Copyright.*Open Cluster|Apache License' -g '**/*.go' || true

echo
echo "Show top of up to 6 files that had matches:"
rg -lP 'SPDX-License-Identifier|SPDX-FileCopyrightText|Copyright.*Open Cluster|Apache License' -g '**/*.go' | head -n 6 | while read -r f; do
  echo "---- $f ----"
  sed -n '1,12p' "$f" || true
  echo
done

echo "-----"
echo "Target file content (pkg/cloudevents/generic/options/mqtt/log.go):"
if [ -f pkg/cloudevents/generic/options/mqtt/log.go ]; then
  sed -n '1,40p' pkg/cloudevents/generic/options/mqtt/log.go || true
else
  echo "FILE NOT FOUND"
fi

echo
echo "Files whose first non-empty line does NOT contain SPDX/License text (first non-empty line check):"
if command -v fd >/dev/null 2>&1; then
  fd -e go --hidden --type f . | while read -r file; do
    first=$(sed -n '1,6p' "$file" | awk 'NF{print; exit}' || true)
    if ! echo "$first" | rg -qP 'SPDX-License-Identifier|SPDX-FileCopyrightText|Copyright|Apache License'; then
      echo "$file"
    fi
  done
else
  find . -type f -name '*.go' | while read -r file; do
    first=$(sed -n '1,6p' "$file" | awk 'NF{print; exit}' || true)
    if ! echo "$first" | rg -qP 'SPDX-License-Identifier|SPDX-FileCopyrightText|Copyright|Apache License'; then
      echo "$file"
    fi
  done
fi

Length of output: 9674


Add repository license/SPDX header to pkg/cloudevents/generic/options/mqtt/log.go

pkg/cloudevents/generic/options/mqtt/log.go currently starts with "package mqtt" and is missing the project's license/SPDX header — add the standard header (copy from pkg/helpers/client.go) to the top to avoid CI header-check failures.

🤖 Prompt for AI Agents
In pkg/cloudevents/generic/options/mqtt/log.go around lines 1 to 1, the file is
missing the project's license/SPDX header; add the standard repository
license/SPDX header at the very top of this file (copy the exact header text
used in pkg/helpers/client.go) so the file begins with that header followed by
the existing "package mqtt" line to satisfy CI header-checks.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
go.mod (1)

3-3: Fix go directive: use major.minor only.

go.mod should declare go 1.24, not 1.24.0. The current value will trip go: parsing go.mod: invalid go version.

-go 1.24.0
+go 1.24
♻️ Duplicate comments (1)
pkg/cloudevents/generic/options/mqtt/log.go (1)

1-1: Add SPDX/license header to satisfy repo CI.

OCM repos expect a license/SPDX header at the top of .go files.

+// SPDX-License-Identifier: Apache-2.0
+
 package mqtt

To verify, run a quick header check in-repo:

#!/bin/bash
set -euo pipefail
first=$(sed -n '1,4p' pkg/cloudevents/generic/options/mqtt/log.go | awk 'NF{print; exit}')
rg -nP 'SPDX-License-Identifier' pkg/cloudevents/generic/options/mqtt/log.go && echo "OK: SPDX found" || { echo "Missing SPDX" >&2; exit 1; }
🧹 Nitpick comments (2)
pkg/cloudevents/generic/options/mqtt/log.go (2)

8-12: Document exported type (or make it unexported).

Add a brief doc comment for PahoDebugLogger to appease linters, or make it pahoDebugLogger if not part of the public API.

- type PahoDebugLogger struct {
+// PahoDebugLogger forwards Paho MQTT debug output to klog at V(4).
+type PahoDebugLogger struct {
 }

3-6: Prefer structured logs + avoid unnecessary formatting when V(4) is disabled.

Guard with Enabled() and use InfoS so operators can filter on a stable message.

 import (
+	"fmt"
 	"github.com/eclipse/paho.golang/paho/log"
 	"k8s.io/klog/v2"
 )
 
 func (l *PahoDebugLogger) Println(v ...interface{}) {
-	klog.V(4).Info(v...)
+	if kv := klog.V(4); kv.Enabled() {
+		kv.InfoS("paho mqtt debug", "msg", fmt.Sprint(v...))
+	}
 }
 
 func (l *PahoDebugLogger) Printf(format string, v ...interface{}) {
-	klog.V(4).Infof(format, v...)
+	if kv := klog.V(4); kv.Enabled() {
+		kv.InfoS("paho mqtt debug", "msg", fmt.Sprintf(format, v...))
+	}
 }

Also applies to: 13-19

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1efea8e and c6c3edd.

⛔ Files ignored due to path filters (29)
  • go.sum is excluded by !**/*.sum
  • vendor/github.com/cloudevents/sdk-go/protocol/mqtt_paho/v2/option.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/binding/test/transformer.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/content_type.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/event_marshal.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/event_unmarshal.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/eventcontext_v03.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/eventcontext_v1.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/extensions.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/protocol/doc.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/protocol/http/protocol.go is excluded by !vendor/**
  • vendor/github.com/eclipse/paho.golang/paho/client.go is excluded by !vendor/**
  • vendor/github.com/eclipse/paho.golang/paho/pinger.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/assertion_compare.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/assertion_format.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/assertion_forward.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/assertion_order.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/assertions.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/doc.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/http_assertions.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/yaml/yaml_custom.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/yaml/yaml_default.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/yaml/yaml_fail.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/require/doc.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/require/require.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/require/require_forward.go is excluded by !vendor/**
  • vendor/golang.org/x/time/rate/rate.go is excluded by !vendor/**
  • vendor/golang.org/x/time/rate/sometimes.go is excluded by !vendor/**
  • vendor/modules.txt is excluded by !vendor/**
📒 Files selected for processing (3)
  • go.mod (4 hunks)
  • pkg/cloudevents/generic/options/mqtt/log.go (1 hunks)
  • pkg/cloudevents/generic/options/mqtt/options.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/cloudevents/generic/options/mqtt/options.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: integration
  • GitHub Check: unit
  • GitHub Check: verify
🔇 Additional comments (1)
go.mod (1)

9-9: Verify dependency bumps across cloudevents/paho/testify/x/time.

These look fine, but please sanity-check downstream compile/tests and update go.sum/vendor as needed.

Recommended local steps:

  • go mod tidy
  • go build ./...
  • go test ./...
  • If repo vendors: make vendor (or hack/verify-vendor.sh)

Also applies to: 12-12, 28-28, 89-89

go.mod Outdated
sigs.k8s.io/structured-merge-diff/v4 v4.6.0 // indirect
)

replace github.com/cloudevents/sdk-go/protocol/mqtt_paho/v2 => github.com/skeeey/ce-sdk-go/protocol/mqtt_paho/v2 v2.0.0-20250917052632-f6859c7a861e
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fork replace must be temporary and annotated (block merge until upstreamed).

This replace pins to a personal fork. Add a clear TEMP comment and ensure we remove/switch to upstream before merge.

+// TEMPORARY: use fork for mqtt debug; remove before merge or switch to upstream once PR lands.
 replace github.com/cloudevents/sdk-go/protocol/mqtt_paho/v2 => github.com/skeeey/ce-sdk-go/protocol/mqtt_paho/v2 v2.0.0-20250917052632-f6859c7a861e
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
replace github.com/cloudevents/sdk-go/protocol/mqtt_paho/v2 => github.com/skeeey/ce-sdk-go/protocol/mqtt_paho/v2 v2.0.0-20250917052632-f6859c7a861e
// TEMPORARY: use fork for mqtt debug; remove before merge or switch to upstream once PR lands.
replace github.com/cloudevents/sdk-go/protocol/mqtt_paho/v2 => github.com/skeeey/ce-sdk-go/protocol/mqtt_paho/v2 v2.0.0-20250917052632-f6859c7a861e
🤖 Prompt for AI Agents
In go.mod at line 103, the replace directive points to a personal fork
(github.com/skeeey/...) which must be temporary; add a clear inline TODO/TEMP
comment above or beside this replace indicating it is a temporary fork, why it’s
needed, the owner/PR tracking upstream, and that it must be removed or switched
to upstream before merging; update project policy or PR description to block
merging until an upstream PR or release exists and add an issue/PR link tracking
the upstreaming effort so reviewers can verify removal prior to merge.

@skeeey
Copy link
Member Author

skeeey commented Sep 17, 2025

depend on cloudevents/sdk-go#1189

Signed-off-by: Wei Liu <liuweixa@redhat.com>
@skeeey skeeey changed the title 🌱 enable mqtt debug info 🌱 enable mqtt logger info Sep 23, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
pkg/cloudevents/generic/options/mqtt/logger.go (3)

8-13: Add brief doc comments for exported types

Keeps golint happy and clarifies intent.

-type PahoErrorLogger struct {
-}
+// PahoErrorLogger routes Paho client error logs to klog at Error level.
+type PahoErrorLogger struct{}
 
-type PahoDebugLogger struct {
-}
+// PahoDebugLogger routes Paho client debug logs to klog at V(4).
+type PahoDebugLogger struct{}

25-31: Guard debug logs with verbosity check to avoid overhead when V(4) is off

This avoids some varargs/formatting work on hot paths.

 func (l *PahoDebugLogger) Println(v ...interface{}) {
-	klog.V(4).Info(v...)
+	if !klog.V(4).Enabled() {
+		return
+	}
+	klog.V(4).Info(v...)
 }
 
 func (l *PahoDebugLogger) Printf(format string, v ...interface{}) {
-	klog.V(4).Infof(format, v...)
+	if !klog.V(4).Enabled() {
+		return
+	}
+	klog.V(4).Infof(format, v...)
 }

17-31: Optional: switch to value receivers to widen method set

Using value receivers lets both T and *T satisfy log.Logger; no behavior change.

-func (l *PahoErrorLogger) Println(v ...interface{}) {
+func (PahoErrorLogger) Println(v ...interface{}) {
 	klog.Error(v...)
 }
 
-func (l *PahoErrorLogger) Printf(format string, v ...interface{}) {
+func (PahoErrorLogger) Printf(format string, v ...interface{}) {
 	klog.Errorf(format, v...)
 }
 
-func (l *PahoDebugLogger) Println(v ...interface{}) {
+func (PahoDebugLogger) Println(v ...interface{}) {
 	klog.V(4).Info(v...)
 }
 
-func (l *PahoDebugLogger) Printf(format string, v ...interface{}) {
+func (PahoDebugLogger) Printf(format string, v ...interface{}) {
 	klog.V(4).Infof(format, v...)
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c6c3edd and e8cbdc9.

⛔ Files ignored due to path filters (98)
  • go.sum is excluded by !**/*.sum
  • vendor/github.com/cloudevents/sdk-go/protocol/mqtt_paho/v2/option.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/binding/test/transformer.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/content_type.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/event_marshal.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/event_unmarshal.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/eventcontext_v03.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/eventcontext_v1.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/event/extensions.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/protocol/doc.go is excluded by !vendor/**
  • vendor/github.com/cloudevents/sdk-go/v2/protocol/http/protocol.go is excluded by !vendor/**
  • vendor/github.com/eclipse/paho.golang/packets/disconnect.go is excluded by !vendor/**
  • vendor/github.com/eclipse/paho.golang/packets/packets.go is excluded by !vendor/**
  • vendor/github.com/eclipse/paho.golang/packets/subscribe.go is excluded by !vendor/**
  • vendor/github.com/eclipse/paho.golang/paho/client.go is excluded by !vendor/**
  • vendor/github.com/eclipse/paho.golang/paho/cp_auth.go is excluded by !vendor/**
  • vendor/github.com/eclipse/paho.golang/paho/pinger.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/assertion_compare.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/assertion_format.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/assertion_forward.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/assertion_order.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/assertions.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/doc.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/http_assertions.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/yaml/yaml_custom.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/yaml/yaml_default.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/assert/yaml/yaml_fail.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/require/doc.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/require/require.go is excluded by !vendor/**
  • vendor/github.com/stretchr/testify/require/require_forward.go is excluded by !vendor/**
  • vendor/golang.org/x/net/http2/frame.go is excluded by !vendor/**
  • vendor/golang.org/x/net/http2/http2.go is excluded by !vendor/**
  • vendor/golang.org/x/net/trace/events.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/mkerrors.sh is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/syscall_darwin.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/syscall_linux.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_386.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_amd64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_arm.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_arm64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_loong64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_mips.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_mips64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_mips64le.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_mipsle.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_ppc.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_ppc64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_ppc64le.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_riscv64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_s390x.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_sparc64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsyscall_darwin_amd64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsyscall_darwin_amd64.s is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsyscall_darwin_arm64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsyscall_darwin_arm64.s is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_386.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_amd64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_arm.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_arm64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_loong64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_mips.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_mips64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_mips64le.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_mipsle.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_ppc.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_ppc64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_ppc64le.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_riscv64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_s390x.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/zsysnum_linux_sparc64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_386.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_amd64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_arm.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_arm64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_loong64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_mips.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_mips64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_mips64le.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_mipsle.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_ppc.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_ppc64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_ppc64le.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_riscv64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_s390x.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux_sparc64.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/windows/registry/key.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/windows/registry/value.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/windows/security_windows.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/windows/syscall_windows.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/windows/types_windows.go is excluded by !vendor/**
  • vendor/golang.org/x/sys/windows/zsyscall_windows.go is excluded by !vendor/**
  • vendor/golang.org/x/term/term_windows.go is excluded by !vendor/**
  • vendor/golang.org/x/term/terminal.go is excluded by !vendor/**
  • vendor/golang.org/x/time/rate/rate.go is excluded by !vendor/**
  • vendor/golang.org/x/time/rate/sometimes.go is excluded by !vendor/**
  • vendor/modules.txt is excluded by !vendor/**
📒 Files selected for processing (3)
  • go.mod (3 hunks)
  • pkg/cloudevents/generic/options/mqtt/logger.go (1 hunks)
  • pkg/cloudevents/generic/options/mqtt/options.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/cloudevents/generic/options/mqtt/options.go
  • go.mod
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: integration
  • GitHub Check: unit
  • GitHub Check: verify
🔇 Additional comments (2)
pkg/cloudevents/generic/options/mqtt/logger.go (2)

14-15: Interface conformance checks: LGTM

Good compile-time assertions guarding API drift.


1-1: Add SPDX/license header to satisfy repo checks

This file is missing the required license/SPDX header and will likely fail CI.

Apply this diff (or copy the exact header used elsewhere in this repo):

+// SPDX-License-Identifier: Apache-2.0
+
 package mqtt

Run to find .go files missing a header:

#!/bin/bash
fd -e go | while read -r f; do
  first_nonempty="$(sed -n '1,6p' "$f" | awk 'NF{print; exit}')"
  if ! grep -Eq 'SPDX-License-Identifier|Copyright|Apache License' <<<"$first_nonempty"; then
    echo "$f"
  fi
done

@skeeey
Copy link
Member Author

skeeey commented Sep 23, 2025

/assign @qiujian16

@skeeey
Copy link
Member Author

skeeey commented Sep 23, 2025

/unhold

@qiujian16
Copy link
Member

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm label Sep 23, 2025
@openshift-ci
Copy link

openshift-ci bot commented Sep 23, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, skeeey

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 023fcb3 into open-cluster-management-io:main Sep 23, 2025
13 checks passed
@skeeey skeeey deleted the mqtt-debug branch October 15, 2025 10:22
@coderabbitai coderabbitai bot mentioned this pull request Nov 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants