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

opentracing: Add flamegraph analysis to opentracing #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

farajidaneshgar
Copy link
Contributor

*The AbstractCalledFunction is updated to maintain a list of children Intervals. Because in Async. Executions the overlapped time of children is calculated twice for self time calculation. So we need to maintain a list of time Intervals of children to compute the intersection of them for Self Time Calculations.

Dependent on eclipse-tracecompass/org.eclipse.tracecompass#45

*The AbstractCalledFunction is updated
 to maintain a list of children Intervals.
 Because in Async. Executions the overlapped
 time of children is calculated twice for self
 time calculation. So we need to maintain a
 list of time Intervals of children to compute
 the intersection of them for Self Time Calculations.

Signed-off-by: Fariba Daneshgar <faraji.daneshgar@gmail.com>
Copy link
Contributor

@arfio arfio left a comment

Choose a reason for hiding this comment

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

This patch adds some interesting features specific to opentracing but it should be changed to use the trace compass core callstack and not the incubator callstack which was recently deprecated.

@@ -181,5 +181,16 @@ public boolean equals(@Nullable Object obj) {
Objects.equals(fParent, other.getParent()) &&
Objects.equals(getSymbol(), other.getSymbol()));
}

private long getChildIntersection(TmfTimeRange chlInt) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This change should be removed as it does not seem to be used anywhere and changes deprecated classes ?

@@ -12,12 +12,20 @@ Require-Bundle: org.eclipse.core.runtime,
org.eclipse.core.resources,
org.eclipse.tracecompass.common.core,
org.eclipse.tracecompass.tmf.core,
org.eclipse.tracecompass.segmentstore.core;bundle-version="3.1.0",
org.eclipse.tracecompass.incubator.callstack.core;bundle-version="0.1.4",
Copy link
Contributor

Choose a reason for hiding this comment

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

Dependencies on org.eclipse.tracecompass.incubator.callstack.core and org.eclipse.tracecompass.incubator.analysis.core should be removed as these projects are mainly deprecated. Other dependencies should not include the bundle-version.

@@ -40,6 +40,18 @@
class="org.eclipse.tracecompass.incubator.internal.opentracing.core.trace.OpenTracingExperiment">
</tracetype>
</module>
<module
Copy link
Contributor

Choose a reason for hiding this comment

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

The indentation and the icon should be corrected.

* @author Fateme Faraji Daneshgar
*
*/
public class AsincCallStackStateProvider extends AbstractTmfStateProvider {
Copy link
Contributor

Choose a reason for hiding this comment

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

AsincCallStackStateProvider -> AsyncCallStackStateProvider

@@ -0,0 +1 @@
package org.eclipse.tracecompass.incubator.opentracing.core.analysis.callstack;
Copy link
Contributor

Choose a reason for hiding this comment

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

This package should be internal and if used elsewhere should add an exception in the manifest file. This file also misses the copyright header.

@@ -0,0 +1,117 @@
package org.eclipse.tracecompass.incubator.opentracing.core.analysis.callstack;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing copyright header.

@@ -0,0 +1,181 @@
package org.eclipse.tracecompass.incubator.opentracing.core.analysis.callstack;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright header

@@ -0,0 +1,178 @@
package org.eclipse.tracecompass.incubator.opentracing.core.analysis.callstack;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright header

@@ -0,0 +1,33 @@
package org.eclipse.tracecompass.incubator.opentracing.core.analysis.callstack;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright header

@@ -0,0 +1,209 @@
package org.eclipse.tracecompass.incubator.opentracing.core.analysis.callstack;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright header

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