Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants