From 86ff86584298ee76444d1d6aa635db112df7bf20 Mon Sep 17 00:00:00 2001
From: Raj Kamal Singh <1133322+raj-k-singh@users.noreply.github.com>
Date: Fri, 29 Dec 2023 18:21:01 +0530
Subject: [PATCH] Fix: logs pipelines: ignore coalesce op when generating nil
 check for add value expressions (#4305)

* chore: panic if agent config recommendation can't be generated

* chore: add case with coalesce op in field nil check generation tests

* fix: ignore expr ast member nodes that contain coalesce op in them
---
 .../app/logparsingpipeline/pipelineBuilder.go              | 7 +++++++
 .../app/logparsingpipeline/pipelineBuilder_test.go         | 7 +++++++
 pkg/query-service/app/opamp/model/agent.go                 | 5 +++--
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/pkg/query-service/app/logparsingpipeline/pipelineBuilder.go b/pkg/query-service/app/logparsingpipeline/pipelineBuilder.go
index aebe4e3890..e569d88a82 100644
--- a/pkg/query-service/app/logparsingpipeline/pipelineBuilder.go
+++ b/pkg/query-service/app/logparsingpipeline/pipelineBuilder.go
@@ -352,6 +352,13 @@ type logFieldsInExprExtractor struct {
 func (v *logFieldsInExprExtractor) Visit(node *ast.Node) {
 	if n, ok := (*node).(*ast.MemberNode); ok {
 		memberRef := n.String()
+
+		// coalesce ops end up as MemberNode right now for some reason.
+		// ignore such member nodes.
+		if strings.Contains(memberRef, "??") {
+			return
+		}
+
 		if strings.HasPrefix(memberRef, "attributes") || strings.HasPrefix(memberRef, "resource") {
 			v.referencedFields = append(v.referencedFields, memberRef)
 		}
diff --git a/pkg/query-service/app/logparsingpipeline/pipelineBuilder_test.go b/pkg/query-service/app/logparsingpipeline/pipelineBuilder_test.go
index d90f74a61e..19ffd4780f 100644
--- a/pkg/query-service/app/logparsingpipeline/pipelineBuilder_test.go
+++ b/pkg/query-service/app/logparsingpipeline/pipelineBuilder_test.go
@@ -698,6 +698,13 @@ func TestMembershipOpInProcessorFieldExpressions(t *testing.T) {
 				Name:    "add3",
 				Field:   `attributes["attrs.test.value"]`,
 				Value:   `EXPR(attributes.test?.value)`,
+			}, {
+				ID:      "add4",
+				Type:    "add",
+				Enabled: true,
+				Name:    "add4",
+				Field:   `attributes["attrs.test.value"]`,
+				Value:   `EXPR((attributes.temp?.request_context?.scraper ?? [nil])[0])`,
 			},
 		},
 	}
diff --git a/pkg/query-service/app/opamp/model/agent.go b/pkg/query-service/app/opamp/model/agent.go
index 758531f345..4bc5f2f3f0 100644
--- a/pkg/query-service/app/opamp/model/agent.go
+++ b/pkg/query-service/app/opamp/model/agent.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/sha256"
+	"fmt"
 	"sync"
 	"time"
 
@@ -276,8 +277,8 @@ func (agent *Agent) processStatusUpdate(
 func (agent *Agent) updateRemoteConfig(configProvider AgentConfigProvider) bool {
 	recommendedConfig, confId, err := configProvider.RecommendAgentConfig([]byte(agent.EffectiveConfig))
 	if err != nil {
-		zap.S().Errorf("could not generate config recommendation for agent %d: %w", agent.ID, err)
-		return false
+		// The server must always recommend a config.
+		panic(fmt.Errorf("could not generate config recommendation for agent %s: %w", agent.ID, err))
 	}
 
 	cfg := protobufs.AgentRemoteConfig{