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

Cleanup usage of TimestampUnixNanos and its API #2549

Merged
merged 1 commit into from
Feb 25, 2021

Conversation

bogdandrutu
Copy link
Member

@bogdandrutu bogdandrutu commented Feb 24, 2021

Updates #2488

Important Changes:

  • Rename pdata.TimestampUnixNanos to pdata.Timestamp
  • Remove pdata.TimestampUnixNanos and helpers, move them to the pdata.Timestamp type.
  • Fix bug around IsZero, this function should return true if the time is January 1, year 1, 00:00:00 UTC not epoch.

@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #2549 (e9a4c2a) into main (7d7ae2e) will decrease coverage by 0.00%.
The diff coverage is 98.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2549      +/-   ##
==========================================
- Coverage   91.69%   91.68%   -0.01%     
==========================================
  Files         266      266              
  Lines       15111    15101      -10     
==========================================
- Hits        13856    13846      -10     
  Misses        871      871              
  Partials      384      384              
Impacted Files Coverage Δ
consumer/pdata/common.go 98.91% <ø> (-0.01%) ⬇️
...r/internal/scraper/cpuscraper/cpu_scraper_linux.go 100.00% <ø> (ø)
...l/scraper/diskscraper/disk_scraper_others_linux.go 100.00% <ø> (ø)
...raper/filesystemscraper/filesystem_scraper_unix.go 100.00% <ø> (ø)
...rnal/scraper/memoryscraper/memory_scraper_linux.go 100.00% <ø> (ø)
...craper/processesscraper/processes_scraper_linux.go 100.00% <ø> (ø)
...al/scraper/processscraper/process_scraper_linux.go 100.00% <ø> (ø)
translator/internaldata/traces_to_oc.go 86.79% <75.00%> (ø)
consumer/pdata/generated_log.go 100.00% <100.00%> (ø)
consumer/pdata/generated_metrics.go 100.00% <100.00%> (ø)
... and 26 more

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 7d7ae2e...e9a4c2a. Read the comment docs.

@tigrannajaryan
Copy link
Member

Looks good, except maybe change of behavior for zero time. We had a bug which was fixed by this. I don't remember what exactly, maybe try digging using git blame to see when was it introduced.

@bogdandrutu
Copy link
Member Author

Even if we fixed other bug, we did that by adding a new bug because IsZero on the time package means year 1 not 1970

@bogdandrutu
Copy link
Member Author

@tigrannajaryan I think it was a simple bug added by myself #1550

Updates open-telemetry#2488

Important Changes:

* Rename pdata.TimestampUnixNanos to pdata.Timestamp
* Remove pdata.TimestampUnixNanos and helpers, move them of the pdata.Timestamp type.
* Fix bug around IsZero, this function should return true if the time is January 1, year 1, 00:00:00 UTC not epoch.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@bogdandrutu bogdandrutu merged commit 0a8a1dc into open-telemetry:main Feb 25, 2021
@bogdandrutu bogdandrutu deleted the timestamp branch February 25, 2021 17:19
This was referenced Mar 11, 2021
This was referenced Mar 15, 2021
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 2024
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