Skip to content

fix: Prevent InvalidTypeIdException when Prometheus metric has 'type'… - #5696

Open
nagendramohan wants to merge 1 commit into
opensearch-project:mainfrom
nagendramohan:fix/promql-type-label-conflict-5684
Open

fix: Prevent InvalidTypeIdException when Prometheus metric has 'type'…#5696
nagendramohan wants to merge 1 commit into
opensearch-project:mainfrom
nagendramohan:fix/promql-type-label-conflict-5684

Conversation

@nagendramohan

Copy link
Copy Markdown

… label

Add @JsonTypeInfo(use = JsonTypeInfo.Id.NONE) on PrometheusResult to override the parent DataSourceResult interface's polymorphic type handling. This prevents Jackson from interpreting a metric label named 'type' as the polymorphic type discriminator, which caused InvalidTypeIdException during deserialization.

The type dispatch is already handled explicitly via switch statement in ExecuteDirectQueryActionResponse, so polymorphic type annotations are not needed on the concrete class.

Resolves #5684

Description

PromQL queries fail with InvalidTypeIdException when a metric contains a label named type. This happens because the DataSourceResult
interface uses @JsonTypeInfo(property = "type") for polymorphic deserialization, and Jackson interprets the metric's type label as the
type discriminator.

Fix: Add @JsonTypeInfo(use = JsonTypeInfo.Id.NONE) on PrometheusResult to override the parent's polymorphic type handling. The type
dispatch is already handled explicitly via switch statement in ExecuteDirectQueryActionResponse, so annotation-based polymorphism is unnecessary on the concrete class.

Includes a regression test with a metric containing "type": "gauge" label.

Related Issues

Resolves #5684

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • New PPL command checklist all confirmed.
  • API changes companion pull request
    created.
  • Commits are signed per the DCO using --signoff or -s.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check
here.

… label

Add @JsonTypeInfo(use = JsonTypeInfo.Id.NONE) on PrometheusResult to
override the parent DataSourceResult interface's polymorphic type
handling. This prevents Jackson from interpreting a metric label named
'type' as the polymorphic type discriminator, which caused
InvalidTypeIdException during deserialization.

The type dispatch is already handled explicitly via switch statement in
ExecuteDirectQueryActionResponse, so polymorphic type annotations are
not needed on the concrete class.

Resolves opensearch-project#5684

Signed-off-by: Nagendra Mohan <nagendramohan1990@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Remove redundant type name annotation

The @JsonTypeName annotation is redundant when @JsonTypeInfo(use =
JsonTypeInfo.Id.NONE) is used. The NONE type info disables polymorphic type
handling, making the type name annotation ineffective. Consider removing
@JsonTypeName to avoid confusion.

direct-query/src/main/java/org/opensearch/sql/directquery/transport/model/datasource/PrometheusResult.java [25-28]

-@JsonTypeName("prometheus")
 @JsonTypeInfo(use = JsonTypeInfo.Id.NONE)
 @JsonIgnoreProperties(ignoreUnknown = true)
 public class PrometheusResult implements DataSourceResult {
Suggestion importance[1-10]: 3

__

Why: While technically correct that @JsonTypeName is ineffective with JsonTypeInfo.Id.NONE, removing it may break compatibility if the type info strategy changes in the future. The annotation causes no harm and may serve as documentation. This is a minor code cleanup suggestion with minimal impact.

Low

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.

[BUG] PromQL queries fail with InvalidTypeIdException when metric has a label named "type"

1 participant