Skip to content
This repository has been archived by the owner on Dec 23, 2023. It is now read-only.

Exporter/OCAgent: Add OcAgentNodeUtils. #1471

Merged
merged 3 commits into from
Sep 26, 2018

Conversation

songy23
Copy link
Contributor

@songy23 songy23 commented Sep 25, 2018

Updates #1408.

Add utilities for detecting and creating Node.
Equivalent to https://github.com/census-ecosystem/opencensus-go-exporter-ocagent/blob/master/nodeinfo.go.

@codecov-io
Copy link

codecov-io commented Sep 25, 2018

Codecov Report

Merging #1471 into master will decrease coverage by 0.05%.
The diff coverage is 74.62%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1471      +/-   ##
============================================
- Coverage     82.23%   82.17%   -0.06%     
- Complexity     1552     1564      +12     
============================================
  Files           239      240       +1     
  Lines          7266     7333      +67     
  Branches        682      687       +5     
============================================
+ Hits           5975     6026      +51     
- Misses         1082     1097      +15     
- Partials        209      210       +1
Impacted Files Coverage Δ Complexity Δ
...census/exporter/trace/ocagent/TraceProtoUtils.java 87.5% <ø> (ø) 32 <0> (ø) ⬇️
...ter/trace/ocagent/OcAgentTraceExporterHandler.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...ensus/exporter/trace/ocagent/OcAgentNodeUtils.java 74.62% <74.62%> (ø) 12 <12> (?)
...census/implcore/trace/export/SpanExporterImpl.java 93.15% <0%> (+1.36%) 8% <0%> (ø) ⬇️

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 8ca4546...64b6ef3. Read the comment docs.

@songy23
Copy link
Contributor Author

songy23 commented Sep 25, 2018

Friendly ping @bogdandrutu

Timestamp censusTimestamp = Timestamp.fromMillis(System.currentTimeMillis());
return Node.newBuilder()
.setIdentifier(getProcessIdentifier(jvmName, censusTimestamp))
.setLibraryInfo(getLibraryInfo(OpenCensusLibraryInformation.VERSION))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this exporter should have it's own version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting using a different version for this exporter in the LibraryInfo message, or using a different version even for releasing to Maven?

All other exporters have the same version as opencensus-api, having a different version only for this exporter may be confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

If I compile the application with opencensus-api version 0.16 and opencensus-exporter-trace-ocagent version 0.17 I think this should be reflected in the proto that we send. So I suggest to have a VERSION string in the ocagent package which we update in the same way we update the one in the core library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sense to me, updated.

@songy23 songy23 force-pushed the agent-exporter-impl-2 branch from 382c21e to 64b6ef3 Compare September 26, 2018 16:53
@songy23
Copy link
Contributor Author

songy23 commented Sep 26, 2018

Thanks!

@songy23 songy23 merged commit 9580d5f into census-instrumentation:master Sep 26, 2018
@songy23 songy23 deleted the agent-exporter-impl-2 branch September 26, 2018 17:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants