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

[processor/transform] Convert between sum and gauge in metric context #29091

Merged
merged 32 commits into from
Nov 20, 2023

Conversation

graphaelli
Copy link
Contributor

@graphaelli graphaelli commented Nov 9, 2023

Description: Allow running OTTL convert_sum_to_gauge and convert_gauge_to_sum in metric context instead of datapoint when processor.transform.ConvertBetweenSumAndGaugeMetricContext is enabled

closes #20773

This is the result of an effort at contribfest at Kubecon NA '23.

Copy link

linux-foundation-easycla bot commented Nov 9, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@github-actions github-actions bot added the processor/transform Transform processor label Nov 9, 2023
@graphaelli
Copy link
Contributor Author

Just realized drafts don't request reviews. Applied the same changes forconvert_gauge_to_sum, renamed the feature gate and added a changelog entry - ready for review now.

@graphaelli graphaelli marked this pull request as ready for review November 16, 2023 00:06
@graphaelli graphaelli requested a review from a team November 16, 2023 00:06
@graphaelli graphaelli changed the title [processor/transform] convert_sum_to_gauge in metric context [processor/transform] Convert between sum and gauge in metric context Nov 16, 2023
Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

Thanks for contributing this! Everything mostly looks good to me.

CHANGELOG.md Outdated Show resolved Hide resolved
graphaelli and others added 11 commits November 17, 2023 08:41
per review feedback
**Description:** This add logic to filter logs based on log conditions
and sent desired logs as event markers to the honeycomb marker api.

**Link to tracking Issue:**
open-telemetry#27666

**Testing:** Unit testing for log exporter and config. Added component
testing to `otelcontribcol`.

**Documentation:** README describing component usage

Screenshot of exported markers showing up in Honeycomb
<img width="1225" alt="Screenshot 2023-11-14 at 1 27 49 PM"
src="https://github.com/open-telemetry/opentelemetry-collector-contrib/assets/35741033/128d689a-cf1e-4959-9df3-6c88248a7fdb">

---------

Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com>
…n-telemetry#29022)

**Description:**
- Added a new watch to the k8s_observer extension for k8s services,
which can be enabled using a new flag "observe_services".
- Discovered entities are transformed into a new endpoint type
`k8s.service`.
- Adjusted the receivercreator to support the new type `k8s.service`


**Link to tracking Issue:**
[open-telemetry#29021](open-telemetry#29021)

**Testing:** Added unit tests analogue to the available tests

**Documentation:** Adjusted readme's of k8s_observer and
receivercreator. Added description of new flags and typers.

**Note:**
Current implementation is working as described in the linked ticket.
Please check the potential discussion points mentioned in the ticket:
open-telemetry#29021 (comment)

---------

Co-authored-by: Antoine Toulme <antoine@toulme.name>
**Description:** Adds new a new `IsDouble` function to facilitate type
checking. Most useful when checking the type of a body to determine if
it needs to be parsed or not.

**Link to tracking Issue:**
open-telemetry#27895

**Testing:** Added unit test

**Documentation:** Updated the func readme.

Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com>
…try#28894)

**Description:** I have observed some behavior on a personal collector
deployment where the EMF Exporter is still returning errors for `NaN`
json marshalling. This was in a prometheus -> emf exporter metrics
pipeline.

I could not find the specific NaN value in the metrics when
troubleshooting the error. I curled the `/metrics` endpoint and also
tried using the logging exporter to try to get more information. I could
not find where the NaN value was coming from so I took another look into
the unit tests and found some possible code paths in which NaNs could
slip though.

**Link to tracking Issue:** Original issue
open-telemetry#26267

**Testing:** Added more unit tests. The summary unit tests got a slight
refactor for two reasons. So I could get ride of the unnecessary
typecasting and so that we could more easily test out different
combinations of quantile values.

I have also added a few more histogram unit tests to just verify that
all combinations of NaN values are being checked on their own.
)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
* Update AAD documentation to use connection string instead of
instrumentation key. Follow up to open-telemetry#28854
* Modified the ingestion version from 2.0 to 2.1

**Link to tracking Issue:** <Issue number if applicable>

**Testing:** <Describe what testing was performed and which tests were
added.>

Existing tests.

Output from manual run

``` json
--------- Transmitting 30 items ---------       {"kind": "exporter", "data_type": "logs", "name": "azuremonitor"}
2023-11-13T10:50:23.886-0800    debug   azuremonitorexporter@v0.88.0/factory.go:139     Telemetry transmitted in 378.439395ms   {"kind": "exporter", "data_type": "logs", "name": "azuremonitor"}
2023-11-13T10:50:23.886-0800    debug   azuremonitorexporter@v0.88.0/factory.go:139     Response: 200   {"kind": "exporter", "data_type": "logs", "name": "azuremonitor"}
2023-11-13T10:50:23.886-0800    debug   azuremonitorexporter@v0.88.0/factory.go:139     Items accepted/received: 30/30 {"kind": "exporter", "data_type": "logs", "name": "azuremonitor"}
```

**Documentation:** <Describe the documentation added.>
* Updated Authentication.md
…emetry#29309)

**Description:** 
Fixes an issue with an incorrect default url. Also fixes issue where
dataset slug was required.

**Link to tracking Issue:** <Issue number if applicable>
Related to
open-telemetry#27666

**Testing:** <Describe what testing was performed and which tests were
added.>
Added new tests and tested manually.

**Documentation:** <Describe the documentation added.>
 Updated up README
**Description:** Update Honeycomb Marker Exporter to alpha status

**Link to tracking Issue:** open-telemetry#27666 

**Testing:** 

**Documentation:**

---------

Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com>
…one function (open-telemetry#28886)

If no functions are exposed, exit with no error.

This change allows to remove `extension/encoding` from the allowlist.
@graphaelli
Copy link
Contributor Author

graphaelli commented Nov 17, 2023

not sure what happened there, thought I just merged main back in. the diff is good

@graphaelli
Copy link
Contributor Author

@evan-bradley thanks for the first pass. Addressed all comments and ready for a review.

@evan-bradley
Copy link
Contributor

@graphaelli Sorry, looks like the go.mod changed. Could you rebase one more time?

@evan-bradley evan-bradley merged commit 0a79aa8 into open-telemetry:main Nov 20, 2023
83 checks passed
@github-actions github-actions bot added this to the next release milestone Nov 20, 2023
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this pull request Nov 24, 2023
…open-telemetry#29091)

**Description:** Allow running OTTL `convert_sum_to_gauge` and
`convert_gauge_to_sum` in metric context instead of datapoint when
`processor.transform.ConvertBetweenSumAndGaugeMetricContext` is enabled

closes open-telemetry#20773

<!--
**Testing:** <Describe what testing was performed and which tests were
added.>

**Documentation:** <Describe the documentation added.>
-->

This is the result of an effort at contribfest at Kubecon NA '23.

---------

Signed-off-by: Alex Boten <aboten@lightstep.com>
Co-authored-by: Faith Chikwekwe <faithchikwekwe01@gmail.com>
Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com>
Co-authored-by: Andreas Thaler <andreas.thaler01@sap.com>
Co-authored-by: Antoine Toulme <antoine@toulme.name>
Co-authored-by: Etienne Pelletier <etienne.pelletier@segment.com>
Co-authored-by: bryan-aguilar <46550959+bryan-aguilar@users.noreply.github.com>
Co-authored-by: Rajkumar Rangaraj <rajrang@microsoft.com>
Co-authored-by: Curtis Robert <crobert@splunk.com>
Co-authored-by: Alex Boten <aboten@lightstep.com>
Co-authored-by: Jacob Marble <jacobmarble@gmail.com>
Co-authored-by: Jon <jonnywamsley@gmail.com>
Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
Co-authored-by: Antoine Toulme <antoine@lunar-ocean.com>
@graphaelli graphaelli deleted the metric-conversion-context branch August 2, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[processor/transform] Metric conversion functions use datapoint context instead of metric context