fix: code panics on very long task names

[nouseforaname]

while most of the values have been truncated, it seems that task names ( including their APP/TASK/ prefix ) are not currently truncated.

e.g. given a sourceType value with length >= 126 and a sourceInstance != "", the previous state of the code would panic when trying to truncate sourceInstance (assuming sourceInstance is the reason we need to truncate):

if len(sourceType)+len(sourceInstance)+3 > 128 {
  sourceInstance = sourceInstance[:(128 - len(sourceType) - 3)]
}

for every sourceType >= 126 the above code will produce a panic.

this tries to lose the least amount of info to keep identification of log sources possible.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Testing performed?

• Unit tests

Checklist:

• This PR is being made against the main branch, or relevant version branch
• I have added testing for my changes

[jorbaum]

Thanks for fixing this. Looks almost ready. I found things worthy addressing.

[nouseforaname]

Thanks for fixing this. Looks almost ready. I found things worthy addressing.

Yes you did. An I thought of a different way of approaching this. It now calculates a ratio and tries to trim both values with the minimal amount of chars while keeping as much info as possible.

[jorbaum]

Thanks for working on this so quickly and adding such a good amount of new tests :) .

I found one regression for sourceInstance == "" and one case where I am quite certain it still panics right now.

Its close to being ready. Please also keep in mind that this code is on a critical path for log forwarding and needs to be performant as in some environment we need to process a lot of logs. But I think the current code is fine as is in that respect. Please think about making it simpler by avoiding floating-point rounding complexity though. But again: it is fine as is.

[jorbaum]

LGTM. I think this is ready for merging as is. I got one last idea to not always truncate instance ID to 36 chars.

[jorbaum]

OPTIONAL: When len(sourceType)+len(sourceInstance) > 125 we now hard-cap sourceInstance to 36 chars even when sourceType is tiny. Think about truncating sourceType first, so we would have the same behavior from before even for edge cases like:

sourceType = "CELL" (4 chars), sourceInstance = a 123-char composite identifier
org-payments-prod.space-checkout-eu.app-fraud-scoring-v2.instance-9b1deb4d-…-2b0d7b3dcb6d.zone-eu10-az3-canary

┌──────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────────────┬──────────────────────────────┐
│          │                                               rendered PROCID                                               │             len              │
├──────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────┤
│ Previous │ [CELL/org-payments-prod.space-checkout-eu.app-fraud-scoring-v2.instance-9b1deb4d-…dcb6d.zone-eu10-az3-cana] │ 128 — keeps 121 of 123 chars │
├──────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────┤
│ Current  │ [CELL/org-payments-prod.space-checkout-eu.]                                                                 │ 43 — hard-capped to 36 chars │
└──────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────┴──────────────────────────────┘

[nouseforaname]

but I don't think that's how this will be populated?

I think sourceType will contain CELL/THE-CELL_NAME while sourceIndex will be /UUID or the /0

at least that's what my understanding of the code is currently. sourceInstance seems to contain the index ( numerical/uuid) but sourceType will contain the task/appname/vmname

[chombium]

@jorbaum

OPTIONAL: When len(sourceType)+len(sourceInstance) > 125 we now hard-cap sourceInstance to 36 chars even when sourceType is tiny. Think about truncating sourceType first, so we would have the same behavior from before even for edge cases like

We don't have to do some complex maths when we truncating the value. Whatever we do, we will never know how someone generates the strings. I would rather document how the transaction works and let the users suffer a bit and notice that they are doing some weird things.

IMO: Simple truncate from behind is all that we need

[jorbaum]

@nouseforaname

but I don't think that's how this will be populated?

Yes. This is just me being overly cautious as both are a free-text string in the end even though they SHOULD follow the format you mentioned. Its just a hypothetical example I made up.

[jorbaum]

LGTM

[chombium]

@nouseforaname, @jorbaum keep up the good work 💪

LGTM