Adds a template to generate modular input and use it in a golang runner #6132

hughesjj · 2025-04-16T17:06:00Z

We've been talking about doing this for a while. This expands on Paulo's work getting an XML reader for modular input and go binary for running the addon instead of the .cmd and .sh files we've historically used.

One of the most common frustrations we've seen is the duplicative logic between windows and linux that was necessary when using .cmd and .sh. As we're now supporting three addons, and looking to potentially introduce flavors, this isn't tenable in the future. It currently requires duplicative testing between windows and linux. Given we can only currently test windows using splunk internal provisioning, this also makes development super annoying since we can only test linux with splunk-docker.

Writing the addon in golang allows us to reuse almost all of the same logic between windows and linux, allows us to use gotest on some of that logic, and given the significantly increased shared logic, we can be more confident that windows will work when running docker tests.

pjanotti

I think it is looking good, will do a more detailed review once it is out of draft. It should have a CI workflow running the tests.

packaging/technical-addon/README.md

pjanotti · 2025-04-23T21:27:52Z

packaging/technical-addon/go.mod

@@ -0,0 +1,17 @@
+module github.com/splunk/otel-technical-addon


I'm not sure if the prefix otel- belongs here.

Fair enough, just technical-addon? Maybe splunk-technical-addon?

replace with splunk-technical-addon

pjanotti · 2025-04-23T21:29:16Z

packaging/technical-addon/internal/modularinput/executor.go

@@ -0,0 +1,120 @@
+package modularinput


nit: files are missing headers with license - isn't this being flagged by addlicense tool? I'm pretty sure we have this in the repo...

Huh, it's not in the paths returned via
go list -f '{{ .Dir }}' ./... , which at least for me only returns results in cmd and internal

Our addlicense misses anything in

pkg

packaging

examples

tests

Maybe we should make fixing this a different issue? I'll manually add licenses to these new files for now

Fixed it locally at least

packaging/technical-addon/internal/modularinput/windows.go

pjanotti · 2025-04-23T23:11:08Z

packaging/technical-addon/internal/packaging/targz.go

+	"path/filepath"
+)
+
+func PackageAddon(sourceDir, outputFile string) error {


I think this is not being used right now... anyway, why do you think this needs to be implemented directly? Portability?

So far it's just for testing, don't think it was used in this review

I was debating not using it for testing and instead having the "build all addons" target using golang to package instead of shell, but not sure. In general all addons would want some way to package an addon for testing, as you need to zip it up to pass an app into a splunk docker container for purposes of installation testing.

Currently, in our shell tests, we have a "repackage" function that takes in the default artifact we vend, extracts it, makes some changes to inputs.conf etc, then repackages it, and passes that to orca or docker. I could see this as a reusable component to do the repackaging part of that process.

The advantage of having this in golang would be portability: no dependency on what your machine has installed or compatibility with specific tools, e.g.: IRCC tar on mac as a different set of parameters them the one on the GH runners.

codecov · 2025-04-25T18:52:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.45%. Comparing base (d43f971) to head (19f09dc).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6132      +/-   ##
==========================================
+ Coverage   36.29%   44.45%   +8.16%     
==========================================
  Files         381      390       +9     
  Lines       26751    27014     +263     
==========================================
+ Hits         9710    12010    +2300     
+ Misses      16087    14161    -1926     
+ Partials      954      843     -111

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…scripts

pjanotti

LGTM, more Qs than suggestions.

pjanotti · 2025-04-29T17:36:10Z

Makefile

+SOURCE_DIR?=$(dir $(abspath $(lastword $(MAKEFILE_LIST))))
+BUILD_DIR?=$(git rev-parse --show-toplevel)


nit: these two are already defined in Makefile.Common

pjanotti · 2025-04-29T17:38:09Z

Makefile

@@ -11,7 +13,7 @@ GIT_SHA=$(shell git rev-parse --short HEAD)
 GOARCH=$(shell go env GOARCH)
 GOOS=$(shell go env GOOS)

-FIND_MOD_ARGS=-type f -name "go.mod"
+FIND_MOD_ARGS=-type f -name "go.mod"  -not -path "./packaging/technical-addon/*"


Q.: any specific reason to exclude it? I mean does it break something? It make sense since it is not shipped, but, I'm curious if there is any other reason for that.

It is also defined in Makefile.Common

The addon has some build dependencies which would break a few build targets (ex gotest-ith-codecov)

Honestly, this was rage-building to fix errors, I'll see if I can clean it up. Maybe my Makefile variables aren't consistent across the files?

... Will clean up in a separate PR if that's okay though

pjanotti · 2025-04-29T17:45:49Z

packaging/technical-addon/cmd/modinput_config_generator/generator.go

+func generateTaModInputConfs(config *modularinput.TemplateData, addonSourceDir string, outDir string) error {
+	// handle README/inputs.conf.spec
+	specTemplatePath := filepath.Join(addonSourceDir, "assets", "inputs.conf.spec.tmpl")
+	if _, err := os.Stat(specTemplatePath); os.IsNotExist(err) {


Q.: just curious: do you know if os.Stat works on Windows? Not a problem if doesn't: it should work under Git Bash anyway.

Given that the generator only needs to run on Linux (this is a build tool, would not touch a customer machine), I'm not sure how much it'd matter.

I think we're only going to build the stuff on linux, at least... testing the generated TA binaries would be done on windows yes, but most of our addons are cross platform anyway and thus should probably be built on a single platform before being tested on others.

But yes, for windows developers, git bash should work

looks like it'd work in windows as well

https://go.dev/src/os/stat_windows.go

pjanotti · 2025-04-29T17:49:05Z

packaging/technical-addon/internal/packaging/targz.go

+	"path/filepath"
+)
+
+func PackageAddon(sourceDir, outputFile string) error {


The advantage of having this in golang would be portability: no dependency on what your machine has installed or compatibility with specific tools, e.g.: IRCC tar on mac as a different set of parameters them the one on the GH runners.

hughesjj force-pushed the ta-add-runner-template branch from e71a1c2 to a9ae62d Compare April 17, 2025 22:10

pjanotti reviewed Apr 23, 2025

View reviewed changes

hughesjj force-pushed the ta-add-runner-template branch from 94faf3d to f2fb552 Compare April 25, 2025 19:09

hughesjj added 25 commits April 25, 2025 14:44

Adds a template to generate modular input and use it in a golang runner

d2b28b6

update examples, adds some validation

201b6af

output json instead

e59535b

adds packaging and tests for such

3274936

fix some test invocation

7ce00fa

more debugging

bacbb39

Get printing working properly

699e5d5

yay

5b7bbe3

test works

4aa0d77

fix nondeterministic output

97de0c6

gofmt

3c02b97

manual tidy

aee9d8a

add addlicense

147b36c

conditional makefile include

7252bec

rename addon

245939a

think this fixes makefile inclusion

d1501c8

remove flag prefix

a40fd23

fix tgz test

37276a6

remove sha check

bc87d55

fix makefile abspath and fix lint

4234d07

fix linting and replacements

d585505

Exlcude addon for now, need to refactor addon makefile and packaging …

2f76c9c

…scripts

include build dir setting here

68b661d

make independent of env vars

f4affd7

gofmt

211b9b0

hughesjj force-pushed the ta-add-runner-template branch from 6016462 to 211b9b0 Compare April 25, 2025 21:45

update workflow to ensure we still check lint/license/etc

ee6a469

hughesjj marked this pull request as ready for review April 25, 2025 21:52

hughesjj requested review from a team as code owners April 25, 2025 21:52

install tools

19f09dc

pjanotti approved these changes Apr 29, 2025

View reviewed changes

hughesjj merged commit 7529376 into main Apr 30, 2025
159 of 161 checks passed

hughesjj deleted the ta-add-runner-template branch April 30, 2025 17:29

github-actions bot locked and limited conversation to collaborators Apr 30, 2025

		@@ -0,0 +1,17 @@
		module github.com/splunk/otel-technical-addon

		SOURCE_DIR?=$(dir $(abspath $(lastword $(MAKEFILE_LIST))))
		BUILD_DIR?=$(git rev-parse --show-toplevel)

Adds a template to generate modular input and use it in a golang runner #6132

Adds a template to generate modular input and use it in a golang runner #6132

Uh oh!

Conversation

hughesjj commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pjanotti left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pjanotti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hughesjj commented Apr 16, 2025 •

edited

Loading

codecov bot commented Apr 25, 2025 •

edited

Loading