🇦🇲Armenia @murz

I'm planning to rework the approach by implementing a custom span builder, because wrapping new span creation in each place looks not very convenient.

Implemented, now instead of the error - with default settings (the endpoint is empty now by default) - it theows a warning instead:

 [success] Module opentelemetry has been installed. (Configure)
 [warning] Fallback OpenTelemetry endpoint http://localhost:4318 is not available. Please set the custom endpoint in the module settings. Parent exception: Export failure: Export retry limit exceeded in line 97 of /var/www/html/web/vendor/open-telemetry/sdk/Common/Export/Http/PsrTransport.php

I reworked the D9 support approach in a separate issue ✨ Move OpenTelemetry logger proxy to a separate module Active - @gaurav_manerkar, please check if it works well for your case.

Yeah, good idea, but requires a bit of rework, which I've done in a separate issue ✨ Move OpenTelemetry logger proxy to a separate module Active - and will finalize in this ticket.

murz → made their first commit to this issue’s fork.

Reworked and covered by unit tests.

Another alternative to this issue is to not use any transpilers, but use pure JS. But with this approach, we will get a problem with missing translations in files, included from the JS side (by import, require). And a workaround for this is to include them from the Drupal side instead, something like this - components/my-component/my-component.component.yml:

name: My Component
status: experimental
group: My Components
props:
  type: object
  properties: {}

libraryOverrides:
  js:
    js/CatalogPage.js: { attributes: { type: 'module', defer: true } }
    # We have to include here all dependencies of the components used in the
    # template to extract translation strings from them by Drupal.
    # @see https://www.drupal.org/project/drupal/issues/3493106
    ../../../js/templates/ProductCatalog.js: { attributes: { type: module } }
    ../../../js/organisms/ProductList.js: { attributes: { type: module } }
    ../../../js/organisms/FacetsForm.js: { attributes: { type: module } }
    ../../../js/molecules/Pager.js: { attributes: { type: module } }
    ../../../js/molecules/ProductCard.js: { attributes: { type: module } }

murz → created an issue.

murz → created an issue.

contrib doesn't have 0.x major versions, it starts with 1.x AFAIK, so numbers starting with 0 wouldn't exist.

They do have :) Here is a pretty fresh example: https://www.drupal.org/project/experience_builder → - version 0.1.0-alpha1

murz → created an issue.

Fixed.

Implemented.

murz → created an issue.

murz → created an issue.

Merged the second MR with post-fixes.

Thanks, @Joyakas, now the implementation is great! Merged

murz → created an issue.

Closing as outdated.

murz → made their first commit to this issue’s fork.

Works well now, thanks! Tested pipeline: https://git.drupalcode.org/project/test_helpers/-/pipelines/352738

Thanks!

.nightwatch-base:
  variables:
    DRUPAL_NIGHTWATCH_SEARCH_DIRECTORY: ./

works, others - nope, see https://git.drupalcode.org/project/test_helpers/-/merge_requests/128/pip...

Started to test, but faced another issue - see 🐛 Nightwatch pipeline started to fail if the module contains Nightwatch commands Active

murz → created an issue.

Implemented.

murz → created an issue.

If you have Drush, you can do this directly from the terminal:

drush sqlq "DELETE FROM semaphore WHERE name='cron'"

Implemented.

In Nightwatch 3.x we can do this using the nested calls of browser.element() like this:

        browser.click(
          this.api
            .element(pager.component)
            .findElement({ selector: pager.item, index: 0 }),
            .findElement({ selector: pager.actionButton }),
        )

And you can upgrade Nightwatch to 3.x even for Drupal 10, so seems it's the better way than re-implementing the wheel on the Test Helpers side.

murz → created an issue.

Implemented two new functions in the HttpClientFactoryStub for mocking responses as stack and dynamically:

  /**
   * Adds a custom response to the stack.
   *
   * @param \Psr\Http\Message\ResponseInterface $response
   *   A response to add.
   */
  public function stubAddCustomResponseToStack(ResponseInterface $response);

  /**
   * Sets a custom handler for the HTTP client.
   *
   * @param callable|null $handler
   *   A custom handler or null to unset the handler.
   */
  public function stubSetCustomHandler(?callable $handler);

Together with this, I added the "stub" prefix to all stub functions, to indicate that they are available only in the stub of the service.

By the way, I got an issue with Fibers after updating to the new OpenTelemetry libraries (1.1+), because they conflict with Drupal Fibers, used in the Renderer service.

So for now I just disabled them, and will investigate the issue later, here is the task: 📌 Make OpenTelemetry work with FiberBoundContextStorage Active

Actually, OpenTelemetry can submit not only traces but also metrics and logs. In the recent release of the Drupal OpenTelemetry module → , I added a submodule "opentelemetry_metrics" that can submit metrics. So, maybe we can reuse this module to get the needed telemetry data?

To not insert into the Drupal Core some telemetry-specific things, we can apply auto-instrumentation https://github.com/open-telemetry/opentelemetry-php-instrumentation and hook the needed functions in the wrapper. But the integration of the auto-instrumentation into the Drupal OpenTelemetry contrib module is still ongoing, see ✨ Support auto-instrumentation Active .

I develop the OpenTelemetry contrib module → for Drupal. It is still in Alpha, but I hope it can help with generating interesting traces and metrics for receiving by Drupal Community as telemetry data.

To not insert into Drupal Core some telemetry-specific things, we can apply auto-instrumentation https://github.com/open-telemetry/opentelemetry-php-instrumentation and hook the needed functions in the wrapper.

And maybe eventually we will bring the opentelemetry as a Drupal Core module, that can be installed by default if the user opts in.

I implemented the StatementExecutionFailureEvent and added tests to check that all works properly.

Also, in the scope of this issue, I reworked the opentelemetry_trace_db and included it in the main opentelemetry module, to simplify things.

it'll be better to switch the user to www-data in the container

Would probably be a BC break as any before_script work would then need to make itself accessible to www-data, or would need to know to sudo if relevant.

Yes, I fully understand that this is a breaking change, but still, it's for the great future, right? :) So, I filled a separate feature request with this idea: ✨ Switch drupalci/php-8.3-ubuntu-apache container to use www-data user instead of root Active

murz → created an issue.

By the way, maybe instead of running the Nightwatch (and phpunit too) using sudo -U www-data, it'll be better to switch the user to www-data in the container, and perform all other commands as the www-data user?

And if we really need to do some actions as root, we can use sudo -E prefix for such commands.

This is a common approach in many pipelines to switch the container to the non-root user, and use sudo when we need root actions.

murz → created an issue.

Created a submodule "opentelemetry_metrics" with the OpenTelemetry Metrics API as a Drupal service opentelemetry.metrics.

Thanks, have simplified the CI file and merged.

murz → made their first commit to this issue’s fork.

Thanks for the testing! Merged.

murz → made their first commit to this issue’s fork.

Added the support of the StatementExecutionFailureEvent, need to cover it by test to be sure that it works well.

murz → made their first commit to this issue’s fork.

Thank you for the feedback! Was busy with other project and had no time to contribute to OpenTelemetry, but now got some time and improved some things:

1. In the issue ✨ Add support for open-telemetry/sdk v.1.1 and make missing dependencies exceptions more clear Active I improved error handling that should prevent the recursion, and clearly describes if the required libraries are missing - please test.

2. Implement covering by functional tests, which should prevent us from broken functionality and regressions on the new versions.

3. About this:

* In SigNoz, I saw each request twice, once with a (request) suffix, not if that's on purpose but seems to add little.

- it's the intended behavior, because Drupal actually starts processing the request later, and spending some time on the initialization of the Drupal Kernel and so on, so it's good to see the full image of what is happening under the hood. Maybe I'll add an option to disable this, will think about it.

4. About this:

* Trace stuff could maybe use percentage settings, e.g. detailed traces for database and future stuff only on 1% of the requests, similar to how blackfire and other APM's operate.

Yes, I have this feature in the roadmap, filled a separate issue about this: ✨ Add the OpenTelemetry Sampler and the sampling rate configuration Active
For now, probably you can already control this using the env variables like this:

export OTEL_TRACES_SAMPLER="traceidratio"
export OTEL_TRACES_SAMPLER_ARG="0.5"

- more details here: https://opentelemetry.io/docs/languages/sdk-configuration/general/

But I will test this more later.

So, I hope that covered all your points, let's close this issue and feel free to fill separate issues about each bug and feature request - it will be easier to track them separately.

murz → created an issue.

I created the opentelemetry_metrics submodule which provides Metrics API - here is the MR https://git.drupalcode.org/project/opentelemetry/-/merge_requests/49

Please review and test it on your side before merging.

I tested this patch and it works well, please merge!

Works well without workarounds, so merging!

murz → created an issue.

Implemented two functions:

• HttpClientFactoryStub::getLastResponse(int $delta) for unit tests.
• thGetLastResponse(int delta) for Nightwatch tests.

Duplicate of 📌 Automated Drupal 11 compatibility fixes for test_helpers Needs review , closing

Fixed the phpcs issue and merged.

Good catch, thanks! This bug was not caught by the internal tests, because the Nightwatch test missed the tags, so I fixed the test too.

Thank you for the patch, good feature!

By the Nightwatch.js convention, we should return the data in the result.value, and also return result.status=0 on success.

I extended the patch in the MR and covered this feature with tests, and merged.

murz → made their first commit to this issue’s fork.

murz → made their first commit to this issue’s fork.

murz → created an issue.

murz → made their first commit to this issue’s fork.

Forgot about this issue, sorry for dat :) Fixed the MR now and merged.

Tested on my tests and it works great, so moving to the "Needs review".

This issue is fixed in the scope of 🐛 Composer tests fail because of invalid Drupal version Fixed .

Reworked MR to the 11.x branch, please review.

Updated the issue body with the new changes, please review.

I make a fix to print stdout output to the console, if the installation fails, please review.

Implemented.

murz → created an issue.

murz → created an issue.

murz → created an issue.

Here is a suggestion for numbering pre-stable release numbers like 0.1.0:

Let's stick to 8001+ for that case, because .80 is used in some software version numbering as alpha, .90 as beta (eg in KDE releases like 5.80.0 - 6.0-alpha1, 5.90.0 - 6.0.0-beta1, 5.95.0 - 6.0.0-beta2, 6.0.0 - 6.0.0 release).

In the 📌 Decide what to do with Drupal::CORE_MINIMUM_SCHEMA_VERSION and surrounding logic Active I'm trying to describe the general standard for numbering hook_update_N related to semantic versioning, and starting new numbering from 10000 fits well into the approach (for the release 1.0.0). The problem is only with the pre-stable release numbers like 0.1.0, but this is not very popular case, so let's stick to 9000 for that case, because 90 is used in some software version numbering as alpha (eg in KDE releases).

Also found an issue that we can't use numbers with leading zeros because it leads to not finding the hook_update function, so if we use hook_update_08001 - it will show the error:

  In update.inc line 357:
    Function commercetools_update_8001() does not exist

So, updated the description again.

Which documentation page or file needs to change?

I think this page: https://www.drupal.org/docs/drupal-apis/update-api/introduction-to-the-u... →

And the inline documentation here: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...