Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pkg/ottl] Allow indexing any slice type #29441

Closed
TylerHelmuth opened this issue Nov 21, 2023 · 12 comments · Fixed by #35581 or #36401
Closed

[pkg/ottl] Allow indexing any slice type #29441

TylerHelmuth opened this issue Nov 21, 2023 · 12 comments · Fixed by #35581 or #36401
Assignees
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed never stale Issues marked with this label will be never staled and automatically removed pkg/ottl priority:p2 Medium

Comments

@TylerHelmuth
Copy link
Member

I was just shown the scenario

set(resource.attributes["my.environment.2"], Split(resource.attributes["host.name"],"-")[1])

that produces an error:

{"kind": "processor", "name": "transform", "pipeline": "logs", "error": "type, []string, does not support int indexing", "statement": "set(resource.attributes[\"my.environment.2\"], Split(resource.attributes[\"host.name\"],\"-\")[1])"}

Although the language does support indexing the return value of the Converter, it only supports indexing pcommon.Slice and []any, but Split returns a []string. We need to make indexing slices more generic. This might need to be its own issue. Related code:

case k.Int != nil:
switch r := result.(type) {
case pcommon.Slice:
if int(*k.Int) >= r.Len() || int(*k.Int) < 0 {
return nil, fmt.Errorf("index %v out of bounds", *k.Int)
}
result = ottlcommon.GetValue(r.At(int(*k.Int)))
case []any:
if int(*k.Int) >= len(r) || int(*k.Int) < 0 {
return nil, fmt.Errorf("index %v out of bounds", *k.Int)
}
result = r[*k.Int]
default:
return nil, fmt.Errorf("type, %T, does not support int indexing", result)
}

Originally posted by @TylerHelmuth in #26108 (comment)

Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@SplitThePotCyrus
Copy link

Yes please, fix for this would be great.

I've been trying to figure out this issue for a half a day, until I found this just now. 😓
Now I have to figure out how to accomplish what I need in a different way.

@TylerHelmuth
Copy link
Member Author

The workaround for this issue is to use cache. set(resource.attributes["my.environment.2"], Split(resource.attributes["host.name"],"-")[1]) becomes:

set(cache["temp"], Split(resource.attributes["host.name"],"-"))
set(resource.attributes["my.environment.2"], cache["temp"][1])

Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

Copy link
Contributor

github-actions bot commented Jul 1, 2024

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Jul 1, 2024
@evan-bradley evan-bradley added never stale Issues marked with this label will be never staled and automatically removed and removed Stale labels Jul 1, 2024
@evan-bradley
Copy link
Contributor

At a minimum, we should achieve type coverage for all types that OTTL supports. If possible, a generic solution would be nice, assuming we can do this with either reflection or some creative type casting.

evan-bradley pushed a commit that referenced this issue Oct 14, 2024
**Description:** Allow indexing string slice type. I can also add
support for other types such as []int if needed. Let me know if this
approach is good enough.

I haven't found a more generic solution, thank you for any feedback.

**Link to tracking Issue:** Fixes  #29441

**Testing:** Unit test

**Documentation:**
@evan-bradley
Copy link
Contributor

#35581 only added support for string slices, we still need support for slices of types that OTTL supports.

@evan-bradley evan-bradley reopened this Oct 14, 2024
@TylerHelmuth TylerHelmuth added help wanted Extra attention is needed good first issue Good for newcomers labels Nov 15, 2024
@znd4
Copy link
Contributor

znd4 commented Nov 15, 2024

I'd like to work on this for contribfest (still cloning the repo though 😢)

@TylerHelmuth
Copy link
Member Author

we still need support for slices of types that OTTL supports

I believe that is:

  • []float64
  • []int64
  • []boolean
  • []byte

@znd4
Copy link
Contributor

znd4 commented Nov 15, 2024

should that be [][]byte (sorry for lack of domain knowledge)

@znd4
Copy link
Contributor

znd4 commented Nov 16, 2024

Also, no need to support int32?

@TylerHelmuth
Copy link
Member Author

@znd4 no need to support int32, OTTL uses int64 and float64 exclusively for ints and floats.

sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this issue Dec 17, 2024
**Description:** Allow indexing string slice type. I can also add
support for other types such as []int if needed. Let me know if this
approach is good enough.

I haven't found a more generic solution, thank you for any feedback.

**Link to tracking Issue:** Fixes  open-telemetry#29441

**Testing:** Unit test

**Documentation:**
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed never stale Issues marked with this label will be never staled and automatically removed pkg/ottl priority:p2 Medium
Projects
None yet
4 participants