pkg/telemetry/meterexporters/otlp_grpc.go (1)

15-21: Avoid appending headers twice

WithHeaders(headers) is included in the base slice and then appended again when len(headers) > 0, so we end up pushing the same option twice. Please keep the add inside the conditional (and drop it from the initial slice) so we only register headers when they actually exist.

pkg/development/coverage/coverage.go (1)

264-275: attributes(): avoid sparse slice and early abort; append matches only

Preallocating len(attributes) and assigning by index leaves empty strings for non-matching entities; also one parse error nukes all results. Build a compact slice and skip bad inputs.

-func attributes(en string, attributes []string) []string {
-    attrs := make([]string, len(attributes))
-    for index, attrStr := range attributes { // Iterate attribute strings
-        a, err := attribute.Attribute(attrStr)
-        if err != nil {
-            return []string{}
-        }
-        if a.GetEntity().GetType() != en {
-            continue
-        }
-        attrs[index] = fmt.Sprintf("%s#%s", a.GetEntity().GetType(), a.GetAttribute()) // Format attribute
-    } // End iteration
-    return attrs // Return attributes
-} // End attributes
+func attributes(en string, attributes []string) []string {
+    attrs := make([]string, 0, len(attributes))
+    for _, attrStr := range attributes { // Iterate attribute strings
+        a, err := attribute.Attribute(attrStr)
+        if err != nil {
+            continue
+        }
+        if a.GetEntity().GetType() != en {
+            continue
+        }
+        attrs = append(attrs, fmt.Sprintf("%s#%s", a.GetEntity().GetType(), a.GetAttribute())) // Format attribute
+    } // End iteration
+    return attrs // Return attributes
+} // End attributes

Based on learnings

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

pkg/development/coverage/coverage.go (2)

1-4: Consider removing or simplifying the basic inline comments.

The inline comments on package declaration and imports are somewhat verbose and don't add significant value beyond what the code already conveys.

Apply this diff if you'd like to simplify:

-package coverage // Coverage analysis package
-import (         // Package imports
-	"fmt"    // Formatting
-	"slices" // Slice operations
+package coverage
+
+import (
+	"fmt"
+	"slices"

---

180-215: Division-by-zero guards successfully address the critical issue.

The explicit zero-denominator checks at lines 199-213 prevent panics when there are no entities, relationships, attributes, or assertions. This resolves the critical issue flagged in the previous review.

However, the variable names could be clearer. totalRelationships actually counts entities (not relationships), and totalCoveredRelationships sums percentages (not counts). Consider renaming for clarity in a follow-up:

entityCount := 0
sumRelationshipPercent := 0
// ... similar for attributes and assertions

This would make the averaging logic more explicit.

Based on learnings

pkg/telemetry/logexporters/otlp.go (1)

1-55: Consider reducing inline comments.

Every line has an inline comment, which can reduce readability. Consider keeping only comments that explain non-obvious logic, complex decisions, or important gotchas.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

pkg/telemetry/logexporters/otlp.go (1)

26-28: Header propagation and secure gRPC TLS are correctly restored.

Both HTTP/gRPC now pass headers; gRPC secure path sets explicit TLS creds. Parity with meter/tracer exporters achieved.

Also applies to: 45-47, 41-44

pkg/telemetry/logexporters/otlp.go (3)

12-15: Normalize protocol and use named constants (defensive + clearer).

Avoid case/typo surprises and magic strings.

Apply:

 import (
   "context"
   "fmt"
+  "strings"
   ...
 )

+const (
+  ProtocolHTTP = "http"
+  ProtocolGRPC = "grpc"
+)

 func NewOTLP(endpoint string, insecure bool, urlpath string, headers map[string]string, protocol string) (*otlplogs.Exporter, error) {
-  switch protocol { // Determine protocol type
+  protocol = strings.ToLower(strings.TrimSpace(protocol))
+  switch protocol { // Determine protocol type
-  case "http": // HTTP protocol configuration
+  case ProtocolHTTP: // HTTP protocol configuration
-  case "grpc": // gRPC protocol configuration
+  case ProtocolGRPC: // gRPC protocol configuration

---

29-31: Use a bounded context for exporter creation.

Guard init against hangs with a small timeout.

Apply:

 import (
   "context"
   "fmt"
+  "time"
   ...
 )

 func NewOTLP(..., protocol string) (*otlplogs.Exporter, error) {
+  ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+  defer cancel()
   switch protocol {
     case "http":
-      httpExporter, httpErr := otlplogs.NewExporter(context.Background(), otlplogs.WithClient(
+      httpExporter, httpErr := otlplogs.NewExporter(ctx, otlplogs.WithClient(
         otlplogshttp.NewClient(httpOptions...),
       ))
     case "grpc":
-      grpcExporter, grpcErr := otlplogs.NewExporter(context.Background(), otlplogs.WithClient(
+      grpcExporter, grpcErr := otlplogs.NewExporter(ctx, otlplogs.WithClient(
         otlplogsgrpc.NewClient(grpcOptions...),
       ))

If adopted, consider aligning meter/tracer exporters too for consistency. Based on relevant code snippets

Also applies to: 48-50

---

56-57: Clarify error message for unsupported protocol.

Make it self-explanatory.

Apply:

-    return nil, fmt.Errorf("unsupported protocol: %s", protocol) // Return error
+    return nil, fmt.Errorf("unsupported protocol: %q (expected \"http\" or \"grpc\")", protocol) // Return error

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro