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

Fix for #629 #725

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

keesschollaart81
Copy link

@keesschollaart81 keesschollaart81 commented Jan 19, 2022

Given the following metrics:

{
    "contexts": [
        {
            "apdexScores": [],
            "context": "MyContext",
            "counters": [],
            "gauges": [
                {
                    "value": 1,
                    "name": "NameOfMetric|PipelineId:1",
                    "tags": {
                        "pipelineId": "1"
                    }
                },
                {
                    "value": 1,
                    "name": "NameOfMetric|PipelineId:2",
                    "tags": {
                        "pipelineId": "2"
                    }
                }
            ],
            "histograms": [],
            "bucketHistograms": [],
            "meters": [],
            "timers": [],
            "bucketTimers": []
        }
    ],
    "timestamp": "2022-01-19T13:29:40.9729471Z"
}

Given the following code...

var filter = new MetricsFilter().WhereTaggedWithKeyValue(new TagKeyValueFilter { { "pipelineId", "2"  } });
var filteredMetrics = metrics?.Filter(filter);

I woulf expect filteredMetrics to have a single gauge value. The current dev branch returns both values as the pipelineId tag (the key) exist for both values. The code I'm changing with this PR is either a bug (#629?) or its behavior is unintuitive.

Given the following metrics:
```json
{
    "contexts": [
        {
            "apdexScores": [],
            "context": "MyContext",
            "counters": [],
            "gauges": [
                {
                    "value": 1,
                    "name": "NameOfMetric|PipelineId:1",
                    "tags": {
                        "pipelineId": "1"
                    }
                },
                {
                    "value": 1,
                    "name": "NameOfMetric|PipelineId:2",
                    "tags": {
                        "pipelineId": "2"
                    }
                }
            ],
            "histograms": [],
            "bucketHistograms": [],
            "meters": [],
            "timers": [],
            "bucketTimers": []
        }
    ],
    "timestamp": "2022-01-19T13:29:40.9729471Z"
}
```

Given the following code...

```cs
var filter = new MetricsFilter().WhereTaggedWithKeyValue(new TagKeyValueFilter { { "pipelineId", "`"  } });
var filteredMetrics = metrics?.Filter(filter);
```

I woulf expect `filteredMetrics` to have a single gauge value. The current dev branch returns both values as the tag-key exist for both values. The code I'm changing with this PR is either a bug (AppMetrics#629?) or its behavior is unintuitive.
@smiron
Copy link

smiron commented Nov 15, 2022

Is there any particular reason or which this change is not merged yet?

@alhardy
Copy link
Collaborator

alhardy commented Nov 15, 2022

Checks failed. Also I've mentioned I no longer have time to maintain the project, and given projects like opentelemetry I'm not sure how much value there is anymore with AppMetrics.

Looking for maintainers to continue to project if still valuable

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.

None yet

3 participants