Add IBM Instana Exporter #378
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #378 +/- ##
============================================
- Coverage 83.42% 82.37% -1.05%
- Complexity 1949 2028 +79
============================================
Files 142 146 +4
Lines 8143 8402 +259
============================================
+ Hits 6793 6921 +128
- Misses 1350 1481 +131 Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| private ?string $agentUuid = null, | ||
| private ?string $agentPid = null | ||
| ) { | ||
| $this->defaultServiceName = ResourceInfoFactory::defaultResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME); |
There was a problem hiding this comment.
It feels inefficient to do this, since it will re-evaluate all configured resource detectors. Have you found that it's possible for a span to not have this resource attribute?
There was a problem hiding this comment.
Yeah I found it mandatory because , if OTEL_SERVICE_NAME is not defined the resource for attribute SERVICE_NAME returns null and not default value of resource which is unknown_service:php
$span->getResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME)
There was a problem hiding this comment.
Can you please ensure license is appropriate per https://github.com/cncf/foundation/blob/main/charter.md#11-ip-policy and matches what other packages have?
| { | ||
| for ($attempt = 0; $attempt < self::ATTEMPTS && !$this->performAnnounce(); $attempt++) { | ||
| self::logDebug('Discovery request failed, attempt ' . (string) $attempt); | ||
| sleep(5); |
There was a problem hiding this comment.
This is blocking. Should it be configurable?
There was a problem hiding this comment.
You are right it should be configurable, I updated the default value as 1 and added it as parameter for transport object.
And improved the sleep distance between consecutive retries.
There was a problem hiding this comment.
Addressed. Added no of attempts as configurable option for instanaTransport layer. default value is set to 1.
| return true; | ||
| } | ||
|
|
||
| sleep(1); |
@brettmc , We are good from polices perspective. We are having Apache2.0 License for this exporter. |
|
@brettmc , @ChrisLightfootWild |
@ChrisLightfootWild , @brettmc , |
@bobstrecansky @Nevay , I am trying to reach @brettmc @ChrisLightfootWild since two weeks. I hope everything is good at their end. Do you think you guys can help me to move this PR forward and reviewing this PR in their absence. |
src/Exporter/Instana/README.md
Outdated
|
|
||
| ## Issues | ||
|
|
||
| This repo is maintained by IBM Instana and is read-only. Issues should be reported as part of standard [Instana product support](https://www.ibm.com/support/pages/instana-support). |
There was a problem hiding this comment.
I think this needs to change. It's not really read-only, this is the primary source of this exporter now, right?
Having contrib packages with an external responsibility hasn't really come up before (well, AWS, but we're not managing that well!), but maybe something like: This exporter is primarily maintained by contributors from IBM. Issues may be raised at <link> - I've added this to discuss at our next SIG meeting, as I think it's worth deciding on and documenting responsibilities and expectations for 3rd party products...
There was a problem hiding this comment.
Thank you @brettmc for looking :-) ,
Yes I agree , this is not read-only now. I'll update the readme as suggested.
|
@brettmc , Did you get a chance to decide on how this PR should move on? |
This PR contributes IBM instana exporter which can be used to send the data to Instana agent.