Skip to content

Fix bug related to event timestamp format. #354

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

Merged

Conversation

derekkraan
Copy link
Contributor

@derekkraan derekkraan commented Feb 3, 2022

The exporter assumes that we are using opentelemetry:timestamp/0, which returns a timestamp in the erlang native format. This commit cleans up use of erlang:system_time/1.

The exporter assumes that we are using `opentelemetry:timestamp/0`,
which returns a timestamp in the erlang native format. This commit
cleans up use of `erlang:system_time/1`.
@derekkraan derekkraan force-pushed the use_opentelemetry_timestamp branch from 903fe6e to eb4bc7c Compare February 3, 2022 10:00
@derekkraan derekkraan changed the title Fix bug related to timestamp format. The exporter assumes that we are using the opentelemetry:timestamp() function, which returns a timestamp in the native format. This commit cleans up use of erlang:system_time/1. Fix bug related to event timestamp format. Feb 3, 2022
@codecov
Copy link

codecov bot commented Feb 3, 2022

Codecov Report

Merging #354 (eb4bc7c) into main (5727b2c) will not change coverage.
The diff coverage is 66.66%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #354   +/-   ##
=======================================
  Coverage   33.59%   33.59%           
=======================================
  Files          56       56           
  Lines        4560     4560           
=======================================
  Hits         1532     1532           
  Misses       3028     3028           
Flag Coverage Δ
api 68.90% <100.00%> (ø)
elixir 16.47% <50.00%> (ø)
erlang 33.54% <33.33%> (ø)
exporter 21.57% <ø> (ø)
sdk 77.13% <0.00%> (ø)
zipkin 2.59% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
apps/opentelemetry/src/otel_events.erl 75.00% <0.00%> (ø)
apps/opentelemetry_api/src/opentelemetry.erl 80.68% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5727b2c...eb4bc7c. Read the comment docs.

@tsloughter
Copy link
Member

Agh, thanks! I vaguely recall meaning to look into the use of timestamps and why there were multiple forms but can't remember why... this is definitely the right fix.

@tsloughter tsloughter merged commit ccc6138 into open-telemetry:main Feb 3, 2022
@derekkraan derekkraan deleted the use_opentelemetry_timestamp branch February 3, 2022 12:32
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.

2 participants