Rename oltp_endpoint to otlp_endpoint to match opentelemetry spec and lib name

[sharkymcdongles]

What this PR does:
Renames oltp_endpoint to otlp_endpoint to match opentelemetry spec and lib name

Which issue(s) this PR fixes:
Fixes #5067

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[yeya24]

Thanks for this change. It makes sense to me.

[alanprot]

I also agree.. thanks for pointing this out… I just wonder if there is a way to keep both configs for some time (maybe hiding from the docs and giving a warn on the logs) so we don’t break customers that are already using this config

[friedrichg]

Thanks!
@alanprot To me otlp support is experimental, we need to put it in v1.x Guarantees as we do with all new risky features.

[yeya24]

I agree this feature is just experimental and we shoulbe be able to change it.

[alanprot]

I agree this feature is just experimental and we shoulbe be able to change it.

Its not marked as experimental though, right?

But iI'm ok either way i guess...

[yeya24]

Maybe we should follow the doc and make sure we are backward compatible.

[friedrichg]

Its not marked as experimental though, right?

No, it's not experimental. That was just a suggestion.

Maybe we should follow the doc and make sure we are backward compatible.

yes, that is the way forward. fyi @sharkymcdongles

it should be backward compatible

[sharkymcdongles]

@friedrichg I will make changes then this week. Cheers!

[sharkymcdongles]

@friedrichg I just made some adjustments hope it is okay. Was a quick and dirty during a too long meeting where I was bored af and wanted something to distract me.

[sharkymcdongles]

Whoopsie daisies, I forgot I am working with Golang and not Python and did some stupid stuff. Fixed.

[sharkymcdongles]

Not really sure what is failing now. Anyone able to help? I didn't touch the compactor, and my documentation looks proper.

[sharkymcdongles]

NM seems it was the newline. Not sure about the compactor test though.

[alanprot]

If both are set OtlpEndpoint should take precedence (this will happen when we are migrating from the old config to the new one)

Something like:

if (len(c.Otel.OtlpEndpoint) > 0) && (len(c.Otel.OltpEndpoint) > 0) {
	level.Warn(util_log.Logger).Log("msg", "DEPRECATED: otel.otlp and otel.oltp both set, using otel.otlp because otel.oltp is deprecated")
}

otlpEndpoint := c.Otel.OltpEndpoint

if len(otlpEndpoint) == 0 {
    otlpEndpoint = c.Otel.OltpEndpoint
}

[sharkymcdongles]

I already did that here: 91

Logic is:

If both are set log warning that both are set and otel.oltp will be removed and set options to use OtlpEndpoint.

If only OltpEndpoint is set then use it and warn as well.

Then once it is fully removed then both the if from 91 and 99 can be removed.

Maybe there is a cleaner way than this if else condition though not sure.

[friedrichg]

test failure is not related

[yeya24]

--- FAIL: TestCompactor_ShouldCompactOnlyShardsOwnedByTheInstanceOnShardingEnabledWithShuffleShardingAndMultipleInstancesRunning (4.72s)
compactor_test.go:1298:
Error Trace: /__w/cortex/cortex/pkg/compactor/compactor_test.go:1298
Error: Should be true
Test: TestCompactor_ShouldCompactOnlyShardsOwnedByTheInstanceOnShardingEnabledWithShuffleShardingAndMultipleInstancesRunning
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x190b629]

This is a flaky test we need to take care of. Will create a separate pr to track