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

Add repr to FilterX Otel objects #484

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

bshifter
Copy link
Member

Axorouter encountered an error:
filterx: unable to repr; from='otel_kvlist', to='string'.

Upon investigating the issue, I found relevant hints in Axorouter’s configuration. The configuration suggests that OTel objects still require a repr method, but currently, a JSON-based workaround is used:

      filterx {
        # Fallback if map-to-text is not defined for the appliance.
        # TODO: OTel based types do not support string() cast, so we fallback for format_json().
        #       Remove that when OTel types start supporting string() cast.
        log.body = string(log.body) ?? format_json(log.body);

To address this, I implemented repr methods for all OTel-related FilterX objects:

  • array
  • kvlist
  • logrecord
  • resource
  • scope

Additionally, I provided light tests to validate repr methods using forced string typecasting.

Copy link
Contributor

@OverOrion OverOrion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MessageToJsonString returns a Status. It might seem overly cautious, but could you please check it's value to make sure the output buffer's contents is valid?

@bshifter bshifter force-pushed the fx-otel-repr branch 3 times, most recently from 8f448e7 to 0387d33 Compare February 3, 2025 16:20
@bshifter bshifter requested a review from OverOrion February 3, 2025 16:22
@bshifter bshifter force-pushed the fx-otel-repr branch 4 times, most recently from 41c8bca to 2affb99 Compare February 3, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants