Capability to manually create spans #956

xBis7 · 2024-09-25T13:53:08Z

opentelemetry-plugin issue: #90

This PR is based on the work done on PR #84.

The patch is adding a new step withNewSpan which allows users to create their own spans.

The option accepts 3 parameters

label
- name of the span, string value
attributes
- list of attributes, same as withSpanAttributes
setAttributesOnlyOnParent
- whether to inherit the defined attributes to children spans or not, boolean value
- all defined attributes are inherited by default

From the above 3 parameters, only the label is required and everything else is optional.

Example usage

withNewSpan(label: 'custom-build-span', attributes: ([
  spanAttribute(key: 'modules', value: '2'),
  spanAttribute(key: 'command', value: 'mvn clean install')
]), setAttributesOnlyOnParent: true) {
  sh './build-module1.sh'
  sh './build-module2.sh'
}

or

withNewSpan(label: 'custom-build-span', attributes: ([
  spanAttribute(key: 'modules', value: '2'),
  spanAttribute(key: 'command', value: 'mvn clean install')
])) {
  sh './build-module1.sh'
  sh './build-module2.sh'
}

or

withNewSpan(label: 'custom-build-span') {
  sh './build-module1.sh'
  sh './build-module2.sh'
}

The new step was named withNewSpan to be consistent with the naming of other steps like withSpanAttributes
For the step execution, SpanAttributeStepExecution was reused
Added a parameter so that the user can control whether the defined attributes should be inherited on children spans of not

Testing done

An integration test was added for testing the overall usage with different pipeline scripts
A unit test was added for testing the changes in MonitorPipelineListener
- It's also testing the code logic for not inheriting the new span attributes to children spans
The patch was also tested manually with jenkins and jaeger

Screenshot from manual testing

Submitter checklist

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue

xBis7 · 2024-09-25T13:55:09Z

@kuisathaverat @cyrille-leclerc Can you take a look at this PR?

It's related to this issue #90

xBis7 · 2024-09-25T13:59:24Z

src/test/java/io/jenkins/plugins/opentelemetry/BaseIntegrationTest.java

+        // install() returns a Future for every plugin. Call get() on the Future so that this line blocks
+        // until the operation is finished and the future is available.
+        jenkinsRule.jenkins.getPluginManager().install(
+            Collections.singletonList("durable-task"), true).get(0).get();


Without this change, most of the integration tests are failing to start.

xBis7 · 2024-09-30T16:51:26Z

@kuisathaverat Did you get a chance to take a look?

v1v

I've just added a comment without reviewing the implementation but the syntax and usability

BTW, thanks so much for your contribution 💯 🙏

v1v · 2024-10-09T08:29:51Z

docs/job-traces.md

+        withNewSpan(label: 'custom-build-span', attributes: ([
+            spanAttribute(key: 'modules', value: '2'),
+            spanAttribute(key: 'command', value: 'mvn clean install')


Out of curiosity, could this be less verbose as a key and value instead of a spanAttribute object?

Something like the below pseudo-code:

withNewSpan(label: 'custom-build-span', attributes: ([ 'modules': '2', 'command': 'mvn clean install'])

The declarative syntax might benefit from a shorter syntax. If users decide to use a bunch of attributes, with the current proposal, there will be 26 chars extra per entry.

What do you think?

Hi @v1v, thanks for the review!

could this be less verbose as a key and value instead of a spanAttribute object?

Not at the moment, but it's doable.

In my current approach, I'm reusing the code for the SpanAttributeStepExecution and it works the same way as in WithSpanAttributesStep which has this constructor

@DataBoundConstructor public WithSpanAttributesStep(List<SpanAttribute> spanAttributes) { this.spanAttributes = spanAttributes; }

for that reason, the WithNewSpanStep also accepts a List<SpanAttribute>.

Because it's annotated with @DataBoundConstructor, you can't overload it but if the parameter type changes to Object attributes, it can also accept a map and that would achieve the behavior that you want. We can convert the map to a List<SpanAttribute> and keep reusing the existing code.

The question is, do we want to make the change only in WithNewSpanStep or should we make it in WithSpanAttributesStep to make the change more global and keep the behavior consistent?

I did some testing and it turns out that it's not that straight forward. Jenkins doesn't like the type-cast from Object to a SpanAttribute element of a List. No issues with using a Map, we can get the values and then create a List<SpanAttribute> but that will break consistency with the other steps.

Hi @v1v, any thoughts on this?

Thanks Christos for your analysis. It's totally fine

kuisathaverat · 2024-10-16T09:05:38Z

Sorry for not getting back to you sooner, but I did not find time to perform the manual test needed to verify that there are no regressions. Also, I want to make a huge pipeline test to verify there are no leaks or performance issues.

xBis7 · 2024-10-16T09:41:33Z

@kuisathaverat Is there something I can do to help? Run some test? I've run manual tests but my use-cases might not match yours.

kuisathaverat · 2024-10-16T17:40:02Z

I need more time. I have compared two "similar" pipelines to see if the memory usage grows, and it does.

A pipeline with 500 steps uses about 450MB of RAM and does not increase memory usage after it is finished. I executed it 10 times.

pipeline {
  agent any
  stages {
    stage('prepare') {
      steps {
        bunchOfSteps(0)
      }
    }
  }
}

def bunchOfSteps(level) {
    if (level < 100) {
        sh "echo level ${level}"
        bunchOfSteps(level + 1)
    }
}

A pipeline with 500 steps and the new step to create spans uses about 850MB of RAM and increases the memory usage after it finishes about 350MB. I executed it one time.

pipeline {
  agent any
  stages {
    stage('prepare') {
      steps {
        bunchOfSteps(0)
      }
    }
  }
}

def bunchOfSteps(level) {
    if (level < 500) {
        withNewSpan(label: 'custom-build-span-' + level, attributes: ([
            spanAttribute(key: 'level', value: level),
            ]), setAttributesOnlyOnParent: true) {
                sh "echo level ${level}"
                bunchOfSteps(level + 1)
            }
    }
}

xBis7 · 2024-10-16T18:04:35Z

It would be interesting to compare the RAM usage between

pipeline {
  agent any
  stages {
    stage('prepare') {
      steps {
        bunchOfSteps(0)
      }
    }
  }
}

def bunchOfSteps(level) {
    if (level < 500) {
        withSpanAttributes([
            spanAttribute(key: 'level', value: level),
            ]) {
                sh "echo level ${level}"
                bunchOfSteps(level + 1)
            }
    }
}

pipeline {
  agent any
  stages {
    stage('prepare') {
      steps {
        bunchOfSteps(0)
      }
    }
  }
}

def bunchOfSteps(level) {
    if (level < 500) {
        withNewSpan(label: 'custom-build-span-' + level, attributes: ([
            spanAttribute(key: 'level', value: level),
            ]), setAttributesOnlyOnParent: true) {
                sh "echo level ${level}"
                bunchOfSteps(level + 1)
            }
    }
}

withSpanAttributes might also increase the RAM usage afterwards.

kuisathaverat · 2024-10-16T19:01:04Z

this one directly crash after 250 steps, but I not see the memory change after several executions

def bunchOfSteps(level) {
    if (level < 500) {
        withSpanAttributes([
            spanAttribute(key: 'level', value: level),
            ]) {
                sh "echo level ${level}"
                bunchOfSteps(level + 1)
            }
    }
}

kuisathaverat · 2024-10-16T19:03:04Z

I am going to test this one too

def bunchOfSteps(level) {
    if (level < 500) {
        withNewSpan(label: 'custom-build-span-' + level, setAttributesOnlyOnParent: true) {
                sh "echo level ${level}"
                bunchOfSteps(level + 1)
            }
    }
}

kuisathaverat · 2024-10-16T19:12:01Z

I am going to change the approach recursivity 500 it is a lite too much for a pipeline

kuisathaverat · 2024-10-16T19:18:26Z

pipeline {
  agent any
  stages {
    stage('prepare') {
      steps {
        bunchOfSteps(0)
      }
    }
  }
}

def bunchOfSteps(level) {
    if (level < 10) {
        recursiveSteps(0)
        bunchOfSteps(level + 1)
    }
}

def recursiveSteps(level){
    if (level < 50) {
        withNewSpan(label: 'custom-build-span-' + level, attributes: ([
            spanAttribute(key: 'level', value: level),
            ]), setAttributesOnlyOnParent: true) {
                sh "echo level ${level}"
                recursiveSteps(level + 1)
            }
    }
}

kuisathaverat · 2024-10-16T19:34:18Z

I am also testing

pipeline {
  agent any
  stages {
    stage('prepare') {
      steps {
      withNewSpan(label: 'custom-build-span', attributes: ([
        spanAttribute(key: 'level', value: 'foo'),
        ]), setAttributesOnlyOnParent: false) {
            bunchOfSteps(0)
        }
      }
    }
  }
}

def bunchOfSteps(level) {
    if (level < 500) {
        sh "echo level ${level}"
        bunchOfSteps(level + 1)
    }
}

kuisathaverat · 2024-10-16T19:34:57Z

I need to grab some head dumps and verify there is nothing weird.

xBis7 added 4 commits September 23, 2024 19:57

Capability to manually create spans

7ed23a3

rename allowedSpanIdList to inheritanceAllowedSpanIdList

6452973

test cleanup

7123b65

testing png

cbc7a79

xBis7 requested a review from kuisathaverat as a code owner September 25, 2024 13:53

Merge branch 'main' into with-new-span-dev

eb1ce00

xBis7 commented Sep 25, 2024

View reviewed changes

xBis7 mentioned this pull request Oct 9, 2024

Capability to manually create spans #90

Open

v1v reviewed Oct 9, 2024

View reviewed changes

Merge branch 'main' into with-new-span-dev

d548dd8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Capability to manually create spans #956

Capability to manually create spans #956

xBis7 commented Sep 25, 2024

xBis7 commented Sep 25, 2024

xBis7 Sep 25, 2024 •

edited

Loading

xBis7 commented Sep 30, 2024

v1v left a comment •

edited

Loading

v1v Oct 9, 2024

xBis7 Oct 9, 2024

xBis7 Oct 10, 2024

xBis7 Oct 16, 2024

v1v Oct 16, 2024

kuisathaverat commented Oct 16, 2024 •

edited

Loading

xBis7 commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024 •

edited

Loading

xBis7 commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024 •

edited

Loading

kuisathaverat commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024

Capability to manually create spans #956

Are you sure you want to change the base?

Capability to manually create spans #956

Conversation

xBis7 commented Sep 25, 2024

Testing done

Submitter checklist

xBis7 commented Sep 25, 2024

xBis7 Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

xBis7 commented Sep 30, 2024

v1v left a comment • edited Loading

Choose a reason for hiding this comment

v1v Oct 9, 2024

Choose a reason for hiding this comment

xBis7 Oct 9, 2024

Choose a reason for hiding this comment

xBis7 Oct 10, 2024

Choose a reason for hiding this comment

xBis7 Oct 16, 2024

Choose a reason for hiding this comment

v1v Oct 16, 2024

Choose a reason for hiding this comment

kuisathaverat commented Oct 16, 2024 • edited Loading

xBis7 commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024 • edited Loading

xBis7 commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024 • edited Loading

kuisathaverat commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024

kuisathaverat commented Oct 16, 2024

xBis7 Sep 25, 2024 •

edited

Loading

v1v left a comment •

edited

Loading

kuisathaverat commented Oct 16, 2024 •

edited

Loading

kuisathaverat commented Oct 16, 2024 •

edited

Loading

kuisathaverat commented Oct 16, 2024 •

edited

Loading