Skip to content

Commit 2230e40

Browse files
ramonskieramonskie
authored
Merge pull request #1239 from kiril-keranov/patch-28
Fix issues in open_telemetry_javaagent and splunk_java_agent frameworks
2 parents f889d52 + b67a21d commit 2230e40

6 files changed

Lines changed: 777 additions & 94 deletions

‎src/java/frameworks/elastic_apm_agent.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,13 @@ func (e *ElasticApmAgentFramework) findElasticApmService() *VCAPService {
144144
return nil
145145
}
146146

147-
if vcapServices.HasService("elastic-apm") {
148-
return vcapServices.GetService("elastic-apm")
147+
service := vcapServices.GetService("elastic-apm")
148+
if service != nil {
149+
return service
149150
}
150-
if vcapServices.HasService("elastic") {
151-
return vcapServices.GetService("elastic")
151+
service = vcapServices.GetService("elastic")
152+
if service != nil {
153+
return service
152154
}
153155

154156
for _, services := range vcapServices {

‎src/java/frameworks/open_telemetry_javaagent.go‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,8 @@ func (o *OpenTelemetryJavaagentFramework) Finalize() error {
8585
vcapServices, _ := GetVCAPServices()
8686

8787
// Try to find service by various patterns
88-
var service *VCAPService
89-
if vcapServices.HasService("otel-collector") {
90-
service = vcapServices.GetService("otel-collector")
91-
}
92-
if service == nil && vcapServices.HasService("opentelemetry") {
88+
service := vcapServices.GetService("otel-collector")
89+
if service == nil {
9390
service = vcapServices.GetService("opentelemetry")
9491
}
9592
if service == nil {
@@ -111,8 +108,9 @@ func (o *OpenTelemetryJavaagentFramework) Finalize() error {
111108
// Set otel.service.name to the application name if not specified in credentials
112109
if _, hasServiceName := service.Credentials["otel.service.name"]; !hasServiceName {
113110
// Use the build directory name as the application name
114-
appName := filepath.Base(o.context.Stager.BuildDir())
115-
javaOpts += fmt.Sprintf(" -Dotel.service.name=%s", appName)
111+
if appName := GetApplicationName(false); appName != "" {
112+
javaOpts += fmt.Sprintf(" -Dotel.service.name=%s", appName)
113+
}
116114
}
117115
}
118116

‎src/java/frameworks/open_telemetry_javaagent_test.go‎

Lines changed: 309 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,321 @@ package frameworks_test
22

33
import (
44
"os"
5+
"path/filepath"
56

7+
"github.com/cloudfoundry/java-buildpack/src/java/common"
8+
"github.com/cloudfoundry/java-buildpack/src/java/frameworks"
9+
"github.com/cloudfoundry/libbuildpack"
610
. "github.com/onsi/ginkgo/v2"
711
. "github.com/onsi/gomega"
812
)
913

10-
var _ = Describe("OpenTelemetryJavaagent", func() {
14+
var _ = Describe("OpenTelemetryJavaagentFramework", func() {
15+
var (
16+
ctx *common.Context
17+
framework *frameworks.OpenTelemetryJavaagentFramework
18+
tmpDir string
19+
depsDir string
20+
)
21+
22+
BeforeEach(func() {
23+
var err error
24+
tmpDir, err = os.MkdirTemp("", "otel-javaagent-test-*")
25+
Expect(err).NotTo(HaveOccurred())
26+
27+
depsDir = filepath.Join(tmpDir, "deps")
28+
err = os.MkdirAll(filepath.Join(depsDir, "0"), 0755)
29+
Expect(err).NotTo(HaveOccurred())
30+
31+
logger := libbuildpack.NewLogger(os.Stdout)
32+
manifest := &libbuildpack.Manifest{}
33+
stager := libbuildpack.NewStager([]string{tmpDir, "", depsDir, "0"}, logger, manifest)
34+
35+
ctx = &common.Context{
36+
Stager: stager,
37+
Manifest: manifest,
38+
Log: logger,
39+
}
40+
41+
framework = frameworks.NewOpenTelemetryJavaagentFramework(ctx)
42+
})
43+
1144
AfterEach(func() {
12-
os.Unsetenv("OTEL_SERVICE_NAME")
13-
os.Unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT")
45+
os.RemoveAll(tmpDir)
46+
os.Unsetenv("VCAP_SERVICES")
1447
})
1548

16-
DescribeTable("configuration environment variables",
17-
func(envVar, expectedValue string) {
18-
os.Setenv(envVar, expectedValue)
19-
Expect(os.Getenv(envVar)).To(Equal(expectedValue))
20-
},
21-
Entry("OTEL_SERVICE_NAME", "OTEL_SERVICE_NAME", "my-service"),
22-
Entry("OTEL_EXPORTER_OTLP_ENDPOINT", "OTEL_EXPORTER_OTLP_ENDPOINT", "https://otel-collector.example.com"),
23-
)
49+
Describe("Detect", func() {
50+
Context("without any service binding", func() {
51+
It("does not detect", func() {
52+
name, err := framework.Detect()
53+
Expect(err).NotTo(HaveOccurred())
54+
Expect(name).To(BeEmpty())
55+
})
56+
})
57+
58+
Context("with otel-collector service label", func() {
59+
It("detects successfully", func() {
60+
os.Setenv("VCAP_SERVICES", `{
61+
"otel-collector": [{
62+
"name": "my-otel",
63+
"label": "otel-collector",
64+
"tags": [],
65+
"credentials": {"endpoint": "http://collector:4318"}
66+
}]
67+
}`)
68+
name, err := framework.Detect()
69+
Expect(err).NotTo(HaveOccurred())
70+
Expect(name).To(Equal("OpenTelemetry Javaagent"))
71+
})
72+
})
73+
74+
Context("with opentelemetry service label", func() {
75+
It("detects successfully", func() {
76+
os.Setenv("VCAP_SERVICES", `{
77+
"opentelemetry": [{
78+
"name": "my-otel",
79+
"label": "opentelemetry",
80+
"tags": [],
81+
"credentials": {}
82+
}]
83+
}`)
84+
name, err := framework.Detect()
85+
Expect(err).NotTo(HaveOccurred())
86+
Expect(name).To(Equal("OpenTelemetry Javaagent"))
87+
})
88+
})
89+
90+
Context("with otel tag", func() {
91+
It("detects via tag", func() {
92+
os.Setenv("VCAP_SERVICES", `{
93+
"user-provided": [{
94+
"name": "my-collector",
95+
"label": "user-provided",
96+
"tags": ["otel"],
97+
"credentials": {}
98+
}]
99+
}`)
100+
name, err := framework.Detect()
101+
Expect(err).NotTo(HaveOccurred())
102+
Expect(name).To(Equal("OpenTelemetry Javaagent"))
103+
})
104+
})
105+
106+
Context("with opentelemetry tag", func() {
107+
It("detects via tag", func() {
108+
os.Setenv("VCAP_SERVICES", `{
109+
"user-provided": [{
110+
"name": "my-collector",
111+
"label": "user-provided",
112+
"tags": ["opentelemetry"],
113+
"credentials": {}
114+
}]
115+
}`)
116+
name, err := framework.Detect()
117+
Expect(err).NotTo(HaveOccurred())
118+
Expect(name).To(Equal("OpenTelemetry Javaagent"))
119+
})
120+
})
121+
122+
Context("with otel-collector tag", func() {
123+
It("detects via tag", func() {
124+
os.Setenv("VCAP_SERVICES", `{
125+
"user-provided": [{
126+
"name": "my-collector",
127+
"label": "user-provided",
128+
"tags": ["otel-collector"],
129+
"credentials": {}
130+
}]
131+
}`)
132+
name, err := framework.Detect()
133+
Expect(err).NotTo(HaveOccurred())
134+
Expect(name).To(Equal("OpenTelemetry Javaagent"))
135+
})
136+
})
137+
138+
Context("with user-provided service matching otel-collector name pattern", func() {
139+
It("detects via name pattern", func() {
140+
os.Setenv("VCAP_SERVICES", `{
141+
"user-provided": [{
142+
"name": "my-otel-collector",
143+
"label": "user-provided",
144+
"tags": [],
145+
"credentials": {}
146+
}]
147+
}`)
148+
name, err := framework.Detect()
149+
Expect(err).NotTo(HaveOccurred())
150+
Expect(name).To(Equal("OpenTelemetry Javaagent"))
151+
})
152+
})
153+
154+
Context("with user-provided service matching otel name pattern", func() {
155+
It("detects via name pattern", func() {
156+
os.Setenv("VCAP_SERVICES", `{
157+
"user-provided": [{
158+
"name": "my-otel-sidecar",
159+
"label": "user-provided",
160+
"tags": [],
161+
"credentials": {}
162+
}]
163+
}`)
164+
name, err := framework.Detect()
165+
Expect(err).NotTo(HaveOccurred())
166+
Expect(name).To(Equal("OpenTelemetry Javaagent"))
167+
})
168+
})
169+
170+
Context("with unrelated service binding", func() {
171+
It("does not detect", func() {
172+
os.Setenv("VCAP_SERVICES", `{
173+
"newrelic": [{
174+
"name": "newrelic-service",
175+
"label": "newrelic",
176+
"tags": ["apm"],
177+
"credentials": {"licenseKey": "key"}
178+
}]
179+
}`)
180+
name, err := framework.Detect()
181+
Expect(err).NotTo(HaveOccurred())
182+
Expect(name).To(BeEmpty())
183+
})
184+
})
185+
186+
Context("with invalid VCAP_SERVICES JSON", func() {
187+
It("does not detect and does not error", func() {
188+
os.Setenv("VCAP_SERVICES", `{invalid}`)
189+
name, err := framework.Detect()
190+
Expect(err).NotTo(HaveOccurred())
191+
Expect(name).To(BeEmpty())
192+
})
193+
})
194+
})
195+
196+
Describe("Finalize", func() {
197+
otelOptsFile := func() string {
198+
return filepath.Join(depsDir, "0", "java_opts", "36_open_telemetry_javaagent.opts")
199+
}
200+
201+
Context("with otel-collector service and otel.* credentials", func() {
202+
It("writes javaagent and otel.* properties to opts file", func() {
203+
os.Setenv("VCAP_SERVICES", `{
204+
"otel-collector": [{
205+
"name": "my-otel",
206+
"label": "otel-collector",
207+
"tags": [],
208+
"credentials": {
209+
"otel.exporter.otlp.endpoint": "http://collector:4318",
210+
"otel.traces.sampler": "always_on"
211+
}
212+
}]
213+
}`)
214+
215+
err := framework.Finalize()
216+
Expect(err).NotTo(HaveOccurred())
217+
218+
data, err := os.ReadFile(otelOptsFile())
219+
Expect(err).NotTo(HaveOccurred())
220+
opts := string(data)
221+
222+
Expect(opts).To(ContainSubstring("-javaagent:$DEPS_DIR/0/open_telemetry_javaagent/opentelemetry-javaagent.jar"))
223+
Expect(opts).To(ContainSubstring("-Dotel.exporter.otlp.endpoint=http://collector:4318"))
224+
Expect(opts).To(ContainSubstring("-Dotel.traces.sampler=always_on"))
225+
})
226+
})
227+
228+
Context("with credentials that do not start with otel.", func() {
229+
It("does not include non-otel credentials as JVM properties", func() {
230+
os.Setenv("VCAP_SERVICES", `{
231+
"otel-collector": [{
232+
"name": "my-otel",
233+
"label": "otel-collector",
234+
"tags": [],
235+
"credentials": {
236+
"otel.exporter.otlp.endpoint": "http://collector:4318",
237+
"username": "admin",
238+
"password": "secret"
239+
}
240+
}]
241+
}`)
242+
243+
err := framework.Finalize()
244+
Expect(err).NotTo(HaveOccurred())
245+
246+
data, err := os.ReadFile(otelOptsFile())
247+
Expect(err).NotTo(HaveOccurred())
248+
opts := string(data)
249+
250+
Expect(opts).NotTo(ContainSubstring("-Dusername="))
251+
Expect(opts).NotTo(ContainSubstring("-Dpassword="))
252+
Expect(opts).To(ContainSubstring("-Dotel.exporter.otlp.endpoint=http://collector:4318"))
253+
})
254+
})
255+
256+
Context("without a service binding", func() {
257+
It("writes only the javaagent flag", func() {
258+
err := framework.Finalize()
259+
Expect(err).NotTo(HaveOccurred())
260+
261+
data, err := os.ReadFile(otelOptsFile())
262+
Expect(err).NotTo(HaveOccurred())
263+
opts := string(data)
264+
265+
Expect(opts).To(ContainSubstring("-javaagent:$DEPS_DIR/0/open_telemetry_javaagent/opentelemetry-javaagent.jar"))
266+
Expect(opts).NotTo(ContainSubstring("-Dotel."))
267+
})
268+
})
269+
270+
Context("with otel.service.name already set in credentials", func() {
271+
It("does not add a second otel.service.name from the app name", func() {
272+
os.Setenv("VCAP_SERVICES", `{
273+
"otel-collector": [{
274+
"name": "my-otel",
275+
"label": "otel-collector",
276+
"tags": [],
277+
"credentials": {
278+
"otel.service.name": "explicit-service-name"
279+
}
280+
}]
281+
}`)
282+
283+
err := framework.Finalize()
284+
Expect(err).NotTo(HaveOccurred())
285+
286+
data, err := os.ReadFile(otelOptsFile())
287+
Expect(err).NotTo(HaveOccurred())
288+
opts := string(data)
289+
290+
Expect(opts).To(ContainSubstring("-Dotel.service.name=explicit-service-name"))
291+
// Only one occurrence of otel.service.name
292+
Expect(countOccurrences(opts, "-Dotel.service.name=")).To(Equal(1))
293+
})
294+
})
295+
296+
Context("runtime jar path uses forward slashes", func() {
297+
It("produces a forward-slash path suitable for the Linux container", func() {
298+
err := framework.Finalize()
299+
Expect(err).NotTo(HaveOccurred())
300+
301+
data, err := os.ReadFile(otelOptsFile())
302+
Expect(err).NotTo(HaveOccurred())
303+
304+
Expect(string(data)).To(ContainSubstring("$DEPS_DIR/0/open_telemetry_javaagent/opentelemetry-javaagent.jar"))
305+
})
306+
})
307+
})
24308
})
309+
310+
// countOccurrences counts non-overlapping occurrences of substr in s.
311+
func countOccurrences(s, substr string) int {
312+
count := 0
313+
for i := 0; i <= len(s)-len(substr); {
314+
if s[i:i+len(substr)] == substr {
315+
count++
316+
i += len(substr)
317+
} else {
318+
i++
319+
}
320+
}
321+
return count
322+
}

‎src/java/frameworks/sealights_agent.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ func (f *SealightsAgentFramework) Finalize() error {
135135

136136
// Find Sealights service
137137
var service *VCAPService
138-
if vcapServices.HasService("sealights") {
139-
service = vcapServices.GetService("sealights")
138+
if svc := vcapServices.GetService("sealights"); svc != nil {
139+
service = svc
140140
} else {
141141
service = vcapServices.GetServiceByNamePattern("sealights")
142142
}

0 commit comments

Comments
 (0)