perf: optimize memory usage by avoiding string-to-byte conversions

[amirmalka]

Overview

This PR optimizes memory usage by avoiding unnecessary memory copies when converting between strings and byte slices.

Changes

• Replace `[]byte(string)` conversions with `unsafe.Slice(unsafe.StringData())` to avoid memory copies in batch processing
• Remove debug logging that was converting `[]byte` to `string` (which also copies memory)
• Add configurable `MAX_OBJECT_SIZE` environment variable to prevent sending oversized objects to the server
• Update Go version from 1.24.1 to 1.25.6

Memory Impact

For large objects (e.g., 100MB), this change eliminates the temporary doubling of memory that occurred during string-to-byte conversions, reducing peak memory usage significantly.

Summary by CodeRabbit

• New Features

  • Configurable object size limit via environment variable to control maximum payloads.

• Improvements

  • Reduced verbose logging of object contents.
  • Pre-send validation to skip and report oversized objects.
  • More efficient handling of object payloads to reduce allocations.

• Chores

  • Updated Go toolchain, build image, and CI workflow to use Go 1.25.6.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Warning

Rate limit exceeded

@amirmalka has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 4 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Adds an environment-based MAX_OBJECT_SIZE guard and Client field; replaces string-to-[]byte casts with unsafe.Slice across adapters and core; reduces logging of payload contents; renames a NewClient parameter; and bumps the Go toolchain and build image and CI setup to 1.25.6.

Changes

Cohort / File(s)	Summary
Backend adapter with size guard adapters/backend/v1/client.go	Added MaxObjectSizeEnvVar, getMaxObjectSizeBytes() and Client.maxObjectSizeBytes *int; NewClient initializes the field (param renamed to skipAlertsFrom); pre-send guard in sendPutObjectMessage skips oversized objects and logs an error; replaced []byte(...) casts with unsafe.Slice(...); removed logging of raw object/patch payloads.
In-cluster adapter conversions adapters/incluster/v1/client.go	Imported unsafe and replaced []byte(string) conversions with unsafe.Slice(unsafe.StringData(...), len(...)) for GetObject, PatchObject, PutObject.
Core synchronizer conversions core/synchronizer.go	Replaced string-to-byte conversions with unsafe.Slice(unsafe.StringData(...), len(...)) in GetObject/PutObject flows; minor log formatting tweak.
Build image and CI/toolchain build/Dockerfile, go.mod, tests/go.mod, .github/workflows/pr-created.yaml	Bumped Go toolchain to 1.25.6; updated build base image tag to golang:1.25.6-trixie; CI setup-go upgraded to v6 and now reads Go version from go.mod; added step to print Go version.

Sequence Diagram(s)

sequenceDiagram
    participant Env as Environment
    participant Client as Backend Client
    participant Producer as MessageProducer
    participant Logger as Logger

    Env->>Client: getMaxObjectSizeBytes() (on NewClient)
    Client->>Client: store maxObjectSizeBytes
    Client->>Client: prepare PutObject payload (unsafe.Slice)
    Client->>Client: if payload.size > maxObjectSizeBytes?
    alt oversized
        Client->>Logger: log error ("skipping oversized object")
        Client--x Producer: do not send
    else within limit
        Client->>Producer: send PutObject message
        Producer->>Logger: log send context (no raw payload)
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibbled bytes and checked the size,
A max from env to keep things wise.
unsafe.Slice sped bytes on their way,
Go 1.25 helps me hop and play.
This rabbit cheers the tidy day! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately reflects the main objective: optimizing memory usage by avoiding string-to-byte conversions, which is the primary focus of the changes across multiple files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[matthyx]

check the empty if

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@adapters/backend/v1/client.go`:
- Around line 41-47: In getMaxObjectSizeBytes(), fix the env var validation and
log wording: when reading MaxObjectSizeEnvVar (env name "MAX_OBJECT_SIZE") keep
the existing strconv.Atoi parse but change the logger.L().Error message to
reference "MAX_OBJECT_SIZE" and include the raw value and error; treat any
parsed intVal <= 0 as invalid (log a warning/error mentioning the non-positive
value and ignore it) and do not return a pointer for invalid values—only return
&intVal for positive integers; otherwise return nil as before.

🧹 Nitpick comments (2)

core/synchronizer.go (2)

251-252: Avoid logging the full payload on unmarshal errors.
Stringifying data forces a large copy and risks leaking sensitive payloads. Prefer logging size (or a truncated prefix).

♻️ Suggested tweak

-		logger.L().Ctx(ctx).Error("cannot unmarshal message", helpers.Error(err), helpers.String("target", "domain.Generic"),
-			helpers.String("data", string(data)))
+		logger.L().Ctx(ctx).Error("cannot unmarshal message", helpers.Error(err), helpers.String("target", "domain.Generic"),
+			helpers.Int("data_size", len(data)))

---

417-419: Remove the no‑op if !s.isClient {} block.
It currently does nothing and reads like a missing guard; remove it or document the intended behavior.

[Copilot]

Pull request overview

This PR aims to optimize memory usage by eliminating memory copies during string-to-byte conversions using unsafe operations. However, the PR contains several critical issues that must be addressed before merging.

Changes:

• Replace string-to-byte conversions with unsafe.Slice(unsafe.StringData()) across multiple adapter implementations
• Add configurable MAX_OBJECT_SIZE environment variable to limit object sizes sent to the server
• Update Go version from 1.24.1 to 1.25.6 in go.mod files and Dockerfile

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
go.mod	Updates Go version to 1.25.6
tests/go.mod	Updates Go version to 1.25.6
build/Dockerfile	Updates Docker base image to golang:1.25.6-trixie
core/synchronizer.go	Adds unsafe import and converts string-to-byte conversions using unsafe operations for GetObject and PutObject events; includes an empty if block
adapters/incluster/v1/client.go	Converts string-to-byte conversions using unsafe operations in batch processing for GetObject, PatchObject, and PutObject
adapters/backend/v1/client.go	Adds MAX_OBJECT_SIZE configuration, converts string-to-byte conversions using unsafe operations, removes verbose debug logging

Comments suppressed due to low confidence (2)

core/synchronizer.go:438

• Using unsafe.Slice(unsafe.StringData()) to convert strings to byte slices is dangerous because it creates a mutable byte slice that points to immutable string data. If the returned byte slice is ever modified (directly or by any function it's passed to), it will cause undefined behavior and potential memory corruption since strings in Go are immutable. While the current code paths appear to only read these slices, this creates a fragile assumption that could easily be violated by future changes. Consider documenting this assumption clearly or using a safer approach that still avoids the copy if performance is critical (e.g., using reflect.StringHeader with proper documentation about the immutability requirement).

					helpers.String("cluster", clientId.Cluster),

core/synchronizer.go:419

• This empty if block appears to be incomplete or accidentally committed. Either remove it entirely or add the intended logic. Empty conditional blocks reduce code maintainability and can confuse other developers.

			var msg domain.PutObject
			err = json.Unmarshal(data, &msg)
			if err != nil {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[matthyx]

incredible the number of places we have this...

[matthyx]

why you want to fix it?

[github-actions]

Summary:

• License scan: failure
• Credentials scan: failure
• Vulnerabilities scan: failure
• Unit test: success
• Go linting: failure